Re: [RFC PATCH 2/8] hw/loader: add load_elf_ram_sym_buf() for in-memory ELF loading

[Narayana Murty N <nnmlinux@linux.ibm.com>]

On 17/08/26 3:57 PM, uverma@linux.ibm.com wrote:
> From: Utkarsh Verma <uverma@linux.ibm.com>
> 
> Add a new public API load_elf_ram_sym_buf() that loads an ELF
> image from a caller-supplied memory buffer rather than a file path.
> The implementation writes the buffer to an anonymous memfd, then
> reuses the existing load_elf32/load_elf64 paths.
> 
> To support per-segment inspection by callers (e.g. detecting VoF
> firmware segments), extend the internal load_elf{32,64} template
> (elf_ops.h.inc) with an optional segment_fn_t callback that is
> invoked once for every PT_LOAD segment after its load address is
> resolved. Returning false from the callback aborts the load with
> ELF_LOAD_FAILED.
> 
> AI-used-for: code
> Signed-off-by: Utkarsh Verma <uverma@linux.ibm.com>
> ---
>   hw/core/loader.c         | 77 ++++++++++++++++++++++++++++++++++++++--
>   include/hw/core/loader.h | 30 ++++++++++++++++
>   include/hw/elf_ops.h.inc | 12 ++++++-
>   3 files changed, 116 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 5cbfba0a86..4e4e9764a3 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -43,6 +43,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/memfd.h"
>   #include "qemu/datadir.h"
>   #include "qemu/error-report.h"
>   #include "qapi/error.h"
> @@ -510,12 +511,14 @@ ssize_t load_elf_ram_sym(const char *filename,
>           ret = load_elf64(filename, fd, elf_note_fn,
>                            translate_fn, translate_opaque, must_swab,
>                            pentry, lowaddr, highaddr, pflags, elf_machine,
> -                         clear_lsb, data_swab, as, load_rom, sym_cb);
> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
> +                         NULL, NULL);
>       } else {
>           ret = load_elf32(filename, fd, elf_note_fn,
>                            translate_fn, translate_opaque, must_swab,
>                            pentry, lowaddr, highaddr, pflags, elf_machine,
> -                         clear_lsb, data_swab, as, load_rom, sym_cb);
> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
> +                         NULL, NULL);
>       }
>   
>       if (ret > 0) {
> @@ -527,6 +530,76 @@ ssize_t load_elf_ram_sym(const char *filename,
>       return ret;
>   }
>   
> +ssize_t load_elf_ram_sym_buf(const uint8_t *buf, size_t buflen,
> +                              uint64_t (*elf_note_fn)(void *, void *, bool),
> +                              uint64_t (*translate_fn)(void *, uint64_t),
> +                              void *translate_opaque, uint64_t *pentry,
> +                              uint64_t *lowaddr, uint64_t *highaddr,
> +                              uint32_t *pflags, int elf_data_order,
> +                              int elf_machine, int clear_lsb, int data_swab,
> +                              AddressSpace *as, bool load_rom,
> +                              symbol_fn_t sym_cb,
> +                              segment_fn_t segment_fn, void *segment_opaque)
> +{
> +    const int host_data_order = HOST_BIG_ENDIAN ? ELFDATA2MSB : ELFDATA2LSB;
> +    int fd, must_swab;
> +    ssize_t ret = ELF_LOAD_FAILED;
> +    uint8_t e_ident[EI_NIDENT];
> +
> +    fd = memfd_create("qemu-elf-buf", MFD_CLOEXEC);

The memfd_create() is Linux-specific.

qemu/memfd.h only exposes the direct memfd_create() declaration for 
Linux configurations, so this also appears to break non-Linux system builds.

Could this use an existing portable QEMU temporary-fd abstraction, or 
alternatively could the ELF loader be factored so the common parser 
accepts an fd/buffer directly?

It seems undesirable for a generic loader API to require Linux memfd 
support.

OR

If it is an arch specific design decision, then we need to document it.

> +    if (fd < 0) {
> +        error_report("load_elf_ram_sym_buf: memfd_create: %s", strerror(errno));
> +        return ELF_LOAD_FAILED;
> +    }
> +
> +    if (write(fd, buf, buflen) != (ssize_t)buflen) {

A single write() is not guaranteed to consume the whole buffer and can 
be interrupted.

Also, if buflen > SSIZE_MAX, the cast used for comparison is problematic.

Please use the appropriate QEMU full-write helper (or loop handling 
short writes/EINTR).

> +        error_report("load_elf_ram_sym_buf: write: %s", strerror(errno));
> +        goto fail;
> +    }
> +
> +    lseek(fd, 0, SEEK_SET);
> +    if (read(fd, e_ident, sizeof(e_ident)) != sizeof(e_ident)) {
> +        goto fail;
> +    }
> +    if (e_ident[0] != ELFMAG0 ||
> +        e_ident[1] != ELFMAG1 ||
> +        e_ident[2] != ELFMAG2 ||
> +        e_ident[3] != ELFMAG3) {
> +        ret = ELF_LOAD_NOT_ELF;
> +        goto fail;
> +    }
> +
> +    if (elf_data_order != ELFDATANONE && elf_data_order != e_ident[EI_DATA]) {
> +        ret = ELF_LOAD_WRONG_ENDIAN;
> +        goto fail;
> +    }
> +
> +    must_swab = host_data_order != e_ident[EI_DATA];
> +
> +    lseek(fd, 0, SEEK_SET);

both lseek call errors need to be handled.

> +    if (e_ident[EI_CLASS] == ELFCLASS64) {
> +        ret = load_elf64("(buffer)", fd, elf_note_fn,
> +                         translate_fn, translate_opaque, must_swab,
> +                         pentry, lowaddr, highaddr, pflags, elf_machine,
> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
> +                         segment_fn, segment_opaque);
> +    } else {
> +        ret = load_elf32("(buffer)", fd, elf_note_fn,
> +                         translate_fn, translate_opaque, must_swab,
> +                         pentry, lowaddr, highaddr, pflags, elf_machine,
> +                         clear_lsb, data_swab, as, load_rom, sym_cb,
> +                         segment_fn, segment_opaque);
> +    }
> +
> +    if (ret > 0) {
> +        debuginfo_report_elf("(buffer)", fd, 0);
> +    }
> +
> + fail:
> +    close(fd);
> +    return ret;
> +}
> +
>   static void bswap_uboot_header(uboot_image_header_t *hdr)
>   {
>   #if !HOST_BIG_ENDIAN
> diff --git a/include/hw/core/loader.h b/include/hw/core/loader.h
> index d9431e8a8d..b67b2ca013 100644
> --- a/include/hw/core/loader.h
> +++ b/include/hw/core/loader.h
> @@ -156,6 +156,36 @@ ssize_t load_elf_ram_sym(const char *filename,
>                            int clear_lsb, int data_swab,
>                            AddressSpace *as, bool load_rom, symbol_fn_t sym_cb);
>   
> +/*
> + * segment_fn_t:
> + * Per-PT_LOAD segment callback for load_elf_ram_sym_buf().
> + */
> +typedef bool (*segment_fn_t)(void *opaque,
> +                             uint64_t paddr, uint64_t vaddr,
> +                             uint64_t filesz, uint64_t memsz);
> +
> +/*
> + * load_elf_ram_sym_buf:
> + * @buf: pointer to an in-memory ELF image
> + * @buflen: size of @buf in bytes
> + * @segment_fn: optional per-PT_LOAD callback
> + * @segment_opaque: opaque data passed to @segment_fn
> + *
> + * Identical to load_elf_ram_sym() but loads from a caller-supplied
> + * memory buffer instead of a file.  All other parameters have the
> + * same meaning as load_elf_ram_sym().
> + */
> +ssize_t load_elf_ram_sym_buf(const uint8_t *buf, size_t buflen,
> +                             uint64_t (*elf_note_fn)(void *, void *, bool),
> +                             uint64_t (*translate_fn)(void *, uint64_t),
> +                             void *translate_opaque, uint64_t *pentry,
> +                             uint64_t *lowaddr, uint64_t *highaddr,
> +                             uint32_t *pflags, int elf_data_order,
> +                             int elf_machine, int clear_lsb, int data_swab,
> +                             AddressSpace *as, bool load_rom,
> +                             symbol_fn_t sym_cb,
> +                             segment_fn_t segment_fn, void *segment_opaque);
> +
>   /** load_elf_as:
>    * Same as load_elf_ram_sym(), but always loads the elf as ROM
>    */
> diff --git a/include/hw/elf_ops.h.inc b/include/hw/elf_ops.h.inc
> index 044e72de2a..a2c9b52bf9 100644
> --- a/include/hw/elf_ops.h.inc
> +++ b/include/hw/elf_ops.h.inc
> @@ -321,7 +321,9 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
>                                     uint32_t *pflags, int elf_machine,
>                                     int clear_lsb, int data_swab,
>                                     AddressSpace *as, bool load_rom,
> -                                  symbol_fn_t sym_cb)
> +                                  symbol_fn_t sym_cb,
> +                                  segment_fn_t segment_fn,
> +                                  void *segment_opaque)
>   {
>       struct elfhdr ehdr;
>       struct elf_phdr *phdr = NULL, *ph;
> @@ -504,6 +506,14 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
>                   addr = ph->p_paddr;
>               }
>   
> +            if (segment_fn) {
> +                if (!segment_fn(segment_opaque, addr, ph->p_vaddr,
> +                                file_size, mem_size)) {
> +                    ret = ELF_LOAD_FAILED;
> +                    goto fail;
> +                }
> +            }
> +
>               if (data_swab) {
>                   elf_word j;
>                   for (j = 0; j < file_size; j += (1 << data_swab)) {

Thanks,
Narayana Murty N