Re: [PATCH] hw/elf_ops.h: switch to ssize_t for elf loader return type

[Luc Michel]

On 10:36 Thu 14 Oct     , Stefano Garzarella wrote:
> On Wed, Oct 06, 2021 at 09:28:39PM +0200, Luc Michel wrote:
> >Until now, int was used as the return type for all the ELF
> >loader related functions. The returned value is the sum of all loaded
> >program headers "MemSize" fields.
> >
> >Because of the overflow check in elf_ops.h, trying to load an ELF bigger
> >than INT_MAX will fail. Switch to ssize_t to remove this limitation.
> >
> >Signed-off-by: Luc Michel <lmichel@kalray.eu>
> >---
> > include/hw/elf_ops.h | 25 +++++++++---------
> > include/hw/loader.h  | 60 ++++++++++++++++++++++----------------------
> > hw/core/loader.c     | 60 +++++++++++++++++++++++---------------------
> > 3 files changed, 74 insertions(+), 71 deletions(-)
> >
> >diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> >index 1c37cec4ae..5c2ea0339e 100644
> >--- a/include/hw/elf_ops.h
> >+++ b/include/hw/elf_ops.h
> >@@ -310,24 +310,25 @@ static struct elf_note *glue(get_elf_note_type, 
> >SZ)(struct elf_note *nhdr,
> >     }
> >
> >     return nhdr;
> > }
> >
> >-static int glue(load_elf, SZ)(const char *name, int fd,
> >-                              uint64_t (*elf_note_fn)(void *, void *, bool),
> >-                              uint64_t (*translate_fn)(void *, uint64_t),
> >-                              void *translate_opaque,
> >-                              int must_swab, uint64_t *pentry,
> >-                              uint64_t *lowaddr, uint64_t *highaddr,
> >-                              uint32_t *pflags, int elf_machine,
> >-                              int clear_lsb, int data_swab,
> >-                              AddressSpace *as, bool load_rom,
> >-                              symbol_fn_t sym_cb)
> >+static ssize_t glue(load_elf, SZ)(const char *name, int fd,
> >+                                  uint64_t (*elf_note_fn)(void *, void *, 
> >bool),
> >+                                  uint64_t (*translate_fn)(void *, 
> >uint64_t),
> >+                                  void *translate_opaque,
> >+                                  int must_swab, uint64_t *pentry,
> >+                                  uint64_t *lowaddr, uint64_t *highaddr,
> >+                                  uint32_t *pflags, int elf_machine,
> >+                                  int clear_lsb, int data_swab,
> >+                                  AddressSpace *as, bool load_rom,
> >+                                  symbol_fn_t sym_cb)
> > {
> >     struct elfhdr ehdr;
> >     struct elf_phdr *phdr = NULL, *ph;
> >-    int size, i, total_size;
> >+    int size, i;
> >+    ssize_t total_size;
> >     elf_word mem_size, file_size, data_offset;
> >     uint64_t addr, low = (uint64_t)-1, high = 0;
> >     GMappedFile *mapped_file = NULL;
> >     uint8_t *data = NULL;
> >     int ret = ELF_LOAD_FAILED;
> 
> Since we assign `total_size` to `ret` and we return `ret`, `ret` should 
> be ssize_t too, right?
Yes you are right I missed this one. I'll send a v2.
Thanks.
 
Luc

> 
> The rest LGTM.
> 
> Thanks,
> Stefano
> 
> 
> 
> To declare a filtering error, please use the following link : 
> https://www.security-mail.net/reporter.php?mid=7f69.6167ec18.9b19a.0&r=lmichel%40kalray.eu&s=sgarzare%40redhat.com&o=Re%3A+%5BPATCH%5D+hw%2Felf_ops.h%3A+switch+to+ssize_t+for+elf+loader+return+type&verdict=C&c=618071aa1c7ceb467a44ac8717a3e44186291f37
> 

--