| Message ID | 1476362903-8795-1-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 13.10.16 at 14:48, <roger.pau@citrix.com> wrote: > @@ -174,8 +171,8 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t pstart) > /* Space to store the size of the elf image */ > sz = sizeof(uint32_t); > > - /* Space for the elf and elf section headers */ > - sz += elf_uval(elf, elf->ehdr, e_ehsize) + > + /* Space for the elf header and elf section headers */ > + sz += offsetof(struct elf_sym_header, elf_header.section) + > ELF_BSDSYM_SECTIONS * elf_uval(elf, elf->ehdr, e_shentsize); You've retained the inconsistency which I had asked to eliminate when commenting on v2. > --- a/xen/include/xen/libelf.h > +++ b/xen/include/xen/libelf.h > @@ -432,6 +432,16 @@ struct elf_dom_parms { > uint64_t virt_kend; > }; > > +/* Number of section header needed in order to fit the SYMTAB and STRTAB. */ > +#define ELF_BSDSYM_SECTIONS 3 > +struct elf_sym_header { > + uint32_t size; > + struct { > + elf_ehdr header; > + elf_shdr section[ELF_BSDSYM_SECTIONS]; > + } elf_header; > +} __attribute__((packed)); This doesn't belong here - it's still an internal structure. At most this might go into libelf-private.h, but I think best would be to keep in the C file, just moving it up (and out of any function) there. And if you were to move it into _any_ header, the comment would need adjustment to make clear what part of the loader this actually is relevant for. Jan
On 13/10/16 13:48, Roger Pau Monne wrote: > diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h > index 90bd8cb..70abbaf 100644 > --- a/xen/include/xen/libelf.h > +++ b/xen/include/xen/libelf.h > @@ -432,6 +432,16 @@ struct elf_dom_parms { > uint64_t virt_kend; > }; > > +/* Number of section header needed in order to fit the SYMTAB and STRTAB. */ > +#define ELF_BSDSYM_SECTIONS 3 > +struct elf_sym_header { > + uint32_t size; > + struct { > + elf_ehdr header; > + elf_shdr section[ELF_BSDSYM_SECTIONS]; > + } elf_header; > +} __attribute__((packed)); __packed please, rather than opencoding it. Also, should be between struct and the structure name. ~Andrew
>>> On 13.10.16 at 15:25, <andrew.cooper3@citrix.com> wrote: > On 13/10/16 13:48, Roger Pau Monne wrote: >> diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h >> index 90bd8cb..70abbaf 100644 >> --- a/xen/include/xen/libelf.h >> +++ b/xen/include/xen/libelf.h >> @@ -432,6 +432,16 @@ struct elf_dom_parms { >> uint64_t virt_kend; >> }; >> >> +/* Number of section header needed in order to fit the SYMTAB and STRTAB. > */ >> +#define ELF_BSDSYM_SECTIONS 3 >> +struct elf_sym_header { >> + uint32_t size; >> + struct { >> + elf_ehdr header; >> + elf_shdr section[ELF_BSDSYM_SECTIONS]; >> + } elf_header; >> +} __attribute__((packed)); > > __packed please, rather than opencoding it. Also, should be between > struct and the structure name. There's no __packed in libxc afaics, and the code here is shared. Jan
diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c index 2626a40..3a83d61 100644 --- a/xen/common/libelf/libelf-loader.c +++ b/xen/common/libelf/libelf-loader.c @@ -21,9 +21,6 @@ #include "libelf-private.h" -/* Number of section header needed in order to fit the SYMTAB and STRTAB. */ -#define ELF_BSDSYM_SECTIONS 3 - /* ------------------------------------------------------------------------ */ elf_errorstatus elf_init(struct elf_binary *elf, const char *image_input, size_t size) @@ -174,8 +171,8 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t pstart) /* Space to store the size of the elf image */ sz = sizeof(uint32_t); - /* Space for the elf and elf section headers */ - sz += elf_uval(elf, elf->ehdr, e_ehsize) + + /* Space for the elf header and elf section headers */ + sz += offsetof(struct elf_sym_header, elf_header.section) + ELF_BSDSYM_SECTIONS * elf_uval(elf, elf->ehdr, e_shentsize); sz = elf_round_up(elf, sz); @@ -253,18 +250,11 @@ static void elf_load_bsdsyms(struct elf_binary *elf) * strtab, so we only need three section headers in our fake ELF * header (first section header is always the undefined section). */ - struct { - uint32_t size; - struct { - elf_ehdr header; - elf_shdr section[ELF_BSDSYM_SECTIONS]; - } __attribute__((packed)) elf_header; - } __attribute__((packed)) header; - + struct elf_sym_header header; ELF_HANDLE_DECL(elf_ehdr) header_handle; - unsigned long shdr_size; + unsigned long shdr_size, ehdr_size; ELF_HANDLE_DECL(elf_shdr) section_handle; - unsigned int link, rc; + unsigned int link, rc, i; elf_ptrval header_base; elf_ptrval elf_header_base; elf_ptrval symtab_base; @@ -394,15 +384,35 @@ do { \ header.size = strtab_base + elf_uval(elf, section_handle, sh_size) - elf_header_base; - /* Load the headers. */ + /* Load the size plus elf header. */ + ehdr_size = offsetof(typeof(header), elf_header.section); rc = elf_load_image(elf, header_base, ELF_REALPTR2PTRVAL(&header), - sizeof(header), sizeof(header)); + ehdr_size, ehdr_size); if ( rc != 0 ) { elf_mark_broken(elf, "unable to load ELF headers into guest memory"); return; } + /* + * Load the section headers. + * + * NB: this _must_ be done one by one, and taking the bitness into account, + * so that the guest can treat this as an array of type Elf{32/64}_Shdr. + */ + for ( i = 0; i < ELF_BSDSYM_SECTIONS; i++ ) + { + rc = elf_load_image(elf, header_base + ehdr_size + shdr_size * i, + ELF_REALPTR2PTRVAL(&header.elf_header.section[i]), + shdr_size, shdr_size); + if ( rc != 0 ) + { + elf_mark_broken(elf, + "unable to load ELF section header into guest memory"); + return; + } + } + /* Remove permissions from elf_memcpy_safe. */ elf->caller_xdest_base = NULL; elf->caller_xdest_size = 0; diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h index 90bd8cb..70abbaf 100644 --- a/xen/include/xen/libelf.h +++ b/xen/include/xen/libelf.h @@ -432,6 +432,16 @@ struct elf_dom_parms { uint64_t virt_kend; }; +/* Number of section header needed in order to fit the SYMTAB and STRTAB. */ +#define ELF_BSDSYM_SECTIONS 3 +struct elf_sym_header { + uint32_t size; + struct { + elf_ehdr header; + elf_shdr section[ELF_BSDSYM_SECTIONS]; + } elf_header; +} __attribute__((packed)); + static inline void elf_xen_feature_set(int nr, uint32_t * addr) { addr[nr >> 5] |= 1 << (nr & 31);