| Message ID | 75232c1c2c16c02b38af785a2a95afa571073c39.1464201427.git.alistair.francis@xilinx.com |
|---|---|
| State | New |
| Headers | show |
On 25 May 2016 at 19:49, Alistair Francis <alistair.francis@xilinx.com> wrote: > If the caller didn't specify an architecture for the ELF machine > the load_elf() function will auto detect it based on the ELF file. > > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > V7: > - Fix typo > > hw/core/loader.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 53e0e41..a8a372d 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -419,6 +419,7 @@ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t), > { > int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED; > uint8_t e_ident[EI_NIDENT]; > + uint16_t e_machine; > > fd = open(filename, O_RDONLY | O_BINARY); > if (fd < 0) { > @@ -451,6 +452,15 @@ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t), > goto fail; > } > > + if (elf_machine < 1) { > + /* The caller didn't specify an ARCH, we can figure it out */ > + lseek(fd, 0x12, SEEK_SET); > + if (read(fd, &e_machine, sizeof(e_machine)) != sizeof(e_machine)) { > + goto fail; > + } > + elf_machine = e_machine; > + } Isn't there an endianness problem here, given that e_machine is a 16 bit field? In load_elf32()/load_elf64() we will byteswap the e_machine field we read off the disk if must_swab is true, which will mean it won't match the value we've read here and not byteswapped. I think you're better off pushing the "allow architecture to be unspecified" support down into load_elf32()/load_elf64(), where it can just become if (elf_machine < 1) { elf_machine = ehdr.e_machine; } once the load_elf code has read and byteswapped the header for you. thanks -- PMM
On Thu, Jun 9, 2016 at 10:38 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 25 May 2016 at 19:49, Alistair Francis <alistair.francis@xilinx.com> wrote: >> If the caller didn't specify an architecture for the ELF machine >> the load_elf() function will auto detect it based on the ELF file. >> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> V7: >> - Fix typo >> >> hw/core/loader.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/hw/core/loader.c b/hw/core/loader.c >> index 53e0e41..a8a372d 100644 >> --- a/hw/core/loader.c >> +++ b/hw/core/loader.c >> @@ -419,6 +419,7 @@ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t), >> { >> int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED; >> uint8_t e_ident[EI_NIDENT]; >> + uint16_t e_machine; >> >> fd = open(filename, O_RDONLY | O_BINARY); >> if (fd < 0) { >> @@ -451,6 +452,15 @@ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t), >> goto fail; >> } >> >> + if (elf_machine < 1) { >> + /* The caller didn't specify an ARCH, we can figure it out */ >> + lseek(fd, 0x12, SEEK_SET); >> + if (read(fd, &e_machine, sizeof(e_machine)) != sizeof(e_machine)) { >> + goto fail; >> + } >> + elf_machine = e_machine; >> + } > > Isn't there an endianness problem here, given that e_machine is a 16 > bit field? In load_elf32()/load_elf64() we will byteswap the e_machine > field we read off the disk if must_swab is true, which will mean it won't > match the value we've read here and not byteswapped. > > I think you're better off pushing the "allow architecture to be > unspecified" support down into load_elf32()/load_elf64(), where > it can just become > > if (elf_machine < 1) { > elf_machine = ehdr.e_machine; > } > > once the load_elf code has read and byteswapped the header for you. Good point, I didn't realise it was the same code for both. I have moved it into load_elf64/load_elf32. Thanks, Alistair > > thanks > -- PMM >
diff --git a/hw/core/loader.c b/hw/core/loader.c index 53e0e41..a8a372d 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -419,6 +419,7 @@ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t), { int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED; uint8_t e_ident[EI_NIDENT]; + uint16_t e_machine; fd = open(filename, O_RDONLY | O_BINARY); if (fd < 0) { @@ -451,6 +452,15 @@ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t), goto fail; } + if (elf_machine < 1) { + /* The caller didn't specify an ARCH, we can figure it out */ + lseek(fd, 0x12, SEEK_SET); + if (read(fd, &e_machine, sizeof(e_machine)) != sizeof(e_machine)) { + goto fail; + } + elf_machine = e_machine; + } + lseek(fd, 0, SEEK_SET); if (e_ident[EI_CLASS] == ELFCLASS64) { ret = load_elf64(filename, fd, translate_fn, translate_opaque, must_swab,
If the caller didn't specify an architecture for the ELF machine the load_elf() function will auto detect it based on the ELF file. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- V7: - Fix typo hw/core/loader.c | 10 ++++++++++ 1 file changed, 10 insertions(+)