On 01/22/2014 04:20 PM, Alexey Kardashevskiy wrote:
> The existing load_elf() just returns -1 if it fails to load ELF. However
> it could be smarter than this and tell more about the failure such as
> wrong endianness or incompatible platform.
>
> This adds additional return codes for wrong architecture, wrong
> endianness and if the image is not ELF at all.
>
> This fixes handling of what load_elf() returns for s390x and moxie
> architectures, other callers just check the return value for <0 and
> this remains unchanged.
>
> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
> ---
> hw/core/loader.c | 12 ++++++------
> hw/s390x/ipl.c | 4 ++--
> include/hw/elf_ops.h | 19 ++++++++++++++-----
> include/hw/loader.h | 5 +++++
> 4 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 0634bee..f510260 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -289,7 +289,7 @@ int load_elf(const char *filename, uint64_t
> (*translate_fn)(void *, uint64_t),
> void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr,
> uint64_t *highaddr, int big_endian, int elf_machine, int
> clear_lsb)
> {
> - int fd, data_order, target_data_order, must_swab, ret;
> + int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED;
> uint8_t e_ident[EI_NIDENT];
>
> fd = open(filename, O_RDONLY | O_BINARY);
> @@ -302,8 +302,10 @@ int load_elf(const char *filename, uint64_t
> (*translate_fn)(void *, uint64_t),
> if (e_ident[0] != ELFMAG0 ||
> e_ident[1] != ELFMAG1 ||
> e_ident[2] != ELFMAG2 ||
> - e_ident[3] != ELFMAG3)
> + e_ident[3] != ELFMAG3) {
> + ret = ELF_LOAD_NOT_ELF;
> goto fail;
> + }
> #ifdef HOST_WORDS_BIGENDIAN
> data_order = ELFDATA2MSB;
> #else
> @@ -317,6 +319,7 @@ int load_elf(const char *filename, uint64_t
> (*translate_fn)(void *, uint64_t),
> }
>
> if (target_data_order != e_ident[EI_DATA]) {
> + ret = ELF_LOAD_WRONG_ENDIAN;
> goto fail;
> }
>
> @@ -329,12 +332,9 @@ int load_elf(const char *filename, uint64_t
> (*translate_fn)(void *, uint64_t),
> pentry, lowaddr, highaddr, elf_machine, clear_lsb);
> }
>
> - close(fd);
> - return ret;
> -
> fail:
> close(fd);
> - return -1;
> + return ret;
> }
>
> static void bswap_uboot_header(uboot_image_header_t *hdr)
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 1a6397b..cff77ad 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -97,10 +97,10 @@ static int s390_ipl_init(SysBusDevice *dev)
> } else {
> kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL,
> NULL, 1, ELF_MACHINE, 0);
> - if (kernel_size == -1) {
> + if (kernel_size < 0) {
> kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
> }
> - if (kernel_size == -1) {
> + if (kernel_size < 0) {
> fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel);
> return -1;
> }
> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> index acc701e..b7e7b36 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/hw/elf_ops.h
> @@ -201,6 +201,7 @@ static int glue(load_elf, SZ)(const char *name, int fd,
> uint64_t addr, low = (uint64_t)-1, high = 0;
> uint8_t *data = NULL;
> char label[128];
> + int ret = ELF_LOAD_FAILED;
>
> if (read(fd, &ehdr, sizeof(ehdr)) != sizeof(ehdr))
> goto fail;
> @@ -210,23 +211,31 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>
> switch (elf_machine) {
> case EM_PPC64:
> - if (EM_PPC64 != ehdr.e_machine)
> + if (EM_PPC64 != ehdr.e_machine) {
> if (EM_PPC != ehdr.e_machine)
A stupid bug here, "{" should go one line down :)
I'll repost if someone tells me that the whole idea makes any sense. Thanks.
--
Alexey
