* [PATCH] hw/elf_ops.h: switch to ssize_t for elf loader return type
@ 2021-10-06 19:28 Luc Michel
2021-10-06 21:09 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Luc Michel @ 2021-10-06 19:28 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, Peter Maydell, Paolo Bonzini,
Philippe Mathieu-Daudé,
Stefano Garzarella
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;
@@ -480,11 +481,11 @@ static int glue(load_elf, SZ)(const char *name, int fd,
}
}
}
}
- if (mem_size > INT_MAX - total_size) {
+ if (mem_size > SSIZE_MAX - total_size) {
ret = ELF_LOAD_TOO_BIG;
goto fail;
}
/* address_offset is hack for kernel images that are
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 81104cb02f..4fa485bd61 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -88,11 +88,11 @@ int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz);
#define ELF_LOAD_FAILED -1
#define ELF_LOAD_NOT_ELF -2
#define ELF_LOAD_WRONG_ARCH -3
#define ELF_LOAD_WRONG_ENDIAN -4
#define ELF_LOAD_TOO_BIG -5
-const char *load_elf_strerror(int error);
+const char *load_elf_strerror(ssize_t error);
/** load_elf_ram_sym:
* @filename: Path of ELF file
* @elf_note_fn: optional function to parse ELF Note type
* passed via @translate_opaque
@@ -126,52 +126,52 @@ const char *load_elf_strerror(int error);
* ELF header and no checks will be carried out against the machine type.
*/
typedef void (*symbol_fn_t)(const char *st_name, int st_info,
uint64_t st_value, uint64_t st_size);
-int load_elf_ram_sym(const char *filename,
+ssize_t load_elf_ram_sym(const char *filename,
+ 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 big_endian, int elf_machine,
+ int clear_lsb, int data_swab,
+ AddressSpace *as, bool load_rom, symbol_fn_t sym_cb);
+
+/** load_elf_ram:
+ * Same as load_elf_ram_sym(), but doesn't allow the caller to specify a
+ * symbol callback function
+ */
+ssize_t load_elf_ram(const char *filename,
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 big_endian, int elf_machine,
- int clear_lsb, int data_swab,
- AddressSpace *as, bool load_rom, symbol_fn_t sym_cb);
-
-/** load_elf_ram:
- * Same as load_elf_ram_sym(), but doesn't allow the caller to specify a
- * symbol callback function
- */
-int load_elf_ram(const char *filename,
- 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 big_endian,
- int elf_machine, int clear_lsb, int data_swab,
- AddressSpace *as, bool load_rom);
+ int big_endian, int elf_machine, int clear_lsb,
+ int data_swab, AddressSpace *as, bool load_rom);
/** load_elf_as:
* Same as load_elf_ram(), but always loads the elf as ROM
*/
-int load_elf_as(const char *filename,
- 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 big_endian,
- int elf_machine, int clear_lsb, int data_swab,
- AddressSpace *as);
+ssize_t load_elf_as(const char *filename,
+ 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 big_endian,
+ int elf_machine, int clear_lsb, int data_swab,
+ AddressSpace *as);
/** load_elf:
* Same as load_elf_as(), but doesn't allow the caller to specify an
* AddressSpace.
*/
-int load_elf(const char *filename,
- 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 big_endian,
- int elf_machine, int clear_lsb, int data_swab);
+ssize_t load_elf(const char *filename,
+ 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 big_endian,
+ int elf_machine, int clear_lsb, int data_swab);
/** load_elf_hdr:
* @filename: Path of ELF file
* @hdr: Buffer to populate with header data. Header data will not be
* filled if set to NULL.
diff --git a/hw/core/loader.c b/hw/core/loader.c
index c623318b73..c7f97fdce8 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -324,11 +324,11 @@ static void *load_at(int fd, off_t offset, size_t size)
#define elf_sword int64_t
#define bswapSZs bswap64s
#define SZ 64
#include "hw/elf_ops.h"
-const char *load_elf_strerror(int error)
+const char *load_elf_strerror(ssize_t error)
{
switch (error) {
case 0:
return "No error";
case ELF_LOAD_FAILED:
@@ -400,62 +400,64 @@ void load_elf_hdr(const char *filename, void *hdr, bool *is64, Error **errp)
fail:
close(fd);
}
/* return < 0 if error, otherwise the number of bytes loaded in memory */
-int load_elf(const char *filename,
- 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 big_endian,
- int elf_machine, int clear_lsb, int data_swab)
+ssize_t load_elf(const char *filename,
+ 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 big_endian,
+ int elf_machine, int clear_lsb, int data_swab)
{
return load_elf_as(filename, elf_note_fn, translate_fn, translate_opaque,
pentry, lowaddr, highaddr, pflags, big_endian,
elf_machine, clear_lsb, data_swab, NULL);
}
/* return < 0 if error, otherwise the number of bytes loaded in memory */
-int load_elf_as(const char *filename,
- 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 big_endian,
- int elf_machine, int clear_lsb, int data_swab, AddressSpace *as)
+ssize_t load_elf_as(const char *filename,
+ 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 big_endian,
+ int elf_machine, int clear_lsb, int data_swab,
+ AddressSpace *as)
{
return load_elf_ram(filename, elf_note_fn, translate_fn, translate_opaque,
pentry, lowaddr, highaddr, pflags, big_endian,
elf_machine, clear_lsb, data_swab, as, true);
}
/* return < 0 if error, otherwise the number of bytes loaded in memory */
-int load_elf_ram(const char *filename,
- 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 big_endian,
- int elf_machine, int clear_lsb, int data_swab,
- AddressSpace *as, bool load_rom)
+ssize_t load_elf_ram(const char *filename,
+ 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 big_endian, int elf_machine, int clear_lsb,
+ int data_swab, AddressSpace *as, bool load_rom)
{
return load_elf_ram_sym(filename, elf_note_fn,
translate_fn, translate_opaque,
pentry, lowaddr, highaddr, pflags, big_endian,
elf_machine, clear_lsb, data_swab, as,
load_rom, NULL);
}
/* return < 0 if error, otherwise the number of bytes loaded in memory */
-int load_elf_ram_sym(const char *filename,
- 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 big_endian, int elf_machine,
- int clear_lsb, int data_swab,
- AddressSpace *as, bool load_rom, symbol_fn_t sym_cb)
+ssize_t load_elf_ram_sym(const char *filename,
+ 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 big_endian, int elf_machine,
+ int clear_lsb, int data_swab,
+ AddressSpace *as, bool load_rom, symbol_fn_t sym_cb)
{
- int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED;
+ int fd, data_order, target_data_order, must_swab;
+ ssize_t ret = ELF_LOAD_FAILED;
uint8_t e_ident[EI_NIDENT];
fd = open(filename, O_RDONLY | O_BINARY);
if (fd < 0) {
perror(filename);
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/elf_ops.h: switch to ssize_t for elf loader return type
2021-10-06 19:28 [PATCH] hw/elf_ops.h: switch to ssize_t for elf loader return type Luc Michel
@ 2021-10-06 21:09 ` Philippe Mathieu-Daudé
2021-10-12 15:08 ` Luc Michel
2021-10-14 8:36 ` Stefano Garzarella
2 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-06 21:09 UTC (permalink / raw)
To: Luc Michel, qemu-devel; +Cc: Peter Maydell, Stefano Garzarella, Paolo Bonzini
On 10/6/21 21:28, 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(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/elf_ops.h: switch to ssize_t for elf loader return type
2021-10-06 19:28 [PATCH] hw/elf_ops.h: switch to ssize_t for elf loader return type Luc Michel
2021-10-06 21:09 ` Philippe Mathieu-Daudé
@ 2021-10-12 15:08 ` Luc Michel
2021-10-14 8:36 ` Stefano Garzarella
2 siblings, 0 replies; 5+ messages in thread
From: Luc Michel @ 2021-10-12 15:08 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Paolo Bonzini, Philippe Mathieu-Daudé,
Stefano Garzarella, qemu-trivial
On 21:28 Wed 06 Oct , 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>
Ping?
Cc'ing qemu-trivial. I guess it's simple enough.
Thanks.
--
Luc
> ---
> 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;
> @@ -480,11 +481,11 @@ static int glue(load_elf, SZ)(const char *name, int fd,
> }
> }
> }
> }
>
> - if (mem_size > INT_MAX - total_size) {
> + if (mem_size > SSIZE_MAX - total_size) {
> ret = ELF_LOAD_TOO_BIG;
> goto fail;
> }
>
> /* address_offset is hack for kernel images that are
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 81104cb02f..4fa485bd61 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -88,11 +88,11 @@ int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz);
> #define ELF_LOAD_FAILED -1
> #define ELF_LOAD_NOT_ELF -2
> #define ELF_LOAD_WRONG_ARCH -3
> #define ELF_LOAD_WRONG_ENDIAN -4
> #define ELF_LOAD_TOO_BIG -5
> -const char *load_elf_strerror(int error);
> +const char *load_elf_strerror(ssize_t error);
>
> /** load_elf_ram_sym:
> * @filename: Path of ELF file
> * @elf_note_fn: optional function to parse ELF Note type
> * passed via @translate_opaque
> @@ -126,52 +126,52 @@ const char *load_elf_strerror(int error);
> * ELF header and no checks will be carried out against the machine type.
> */
> typedef void (*symbol_fn_t)(const char *st_name, int st_info,
> uint64_t st_value, uint64_t st_size);
>
> -int load_elf_ram_sym(const char *filename,
> +ssize_t load_elf_ram_sym(const char *filename,
> + 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 big_endian, int elf_machine,
> + int clear_lsb, int data_swab,
> + AddressSpace *as, bool load_rom, symbol_fn_t sym_cb);
> +
> +/** load_elf_ram:
> + * Same as load_elf_ram_sym(), but doesn't allow the caller to specify a
> + * symbol callback function
> + */
> +ssize_t load_elf_ram(const char *filename,
> 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 big_endian, int elf_machine,
> - int clear_lsb, int data_swab,
> - AddressSpace *as, bool load_rom, symbol_fn_t sym_cb);
> -
> -/** load_elf_ram:
> - * Same as load_elf_ram_sym(), but doesn't allow the caller to specify a
> - * symbol callback function
> - */
> -int load_elf_ram(const char *filename,
> - 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 big_endian,
> - int elf_machine, int clear_lsb, int data_swab,
> - AddressSpace *as, bool load_rom);
> + int big_endian, int elf_machine, int clear_lsb,
> + int data_swab, AddressSpace *as, bool load_rom);
>
> /** load_elf_as:
> * Same as load_elf_ram(), but always loads the elf as ROM
> */
> -int load_elf_as(const char *filename,
> - 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 big_endian,
> - int elf_machine, int clear_lsb, int data_swab,
> - AddressSpace *as);
> +ssize_t load_elf_as(const char *filename,
> + 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 big_endian,
> + int elf_machine, int clear_lsb, int data_swab,
> + AddressSpace *as);
>
> /** load_elf:
> * Same as load_elf_as(), but doesn't allow the caller to specify an
> * AddressSpace.
> */
> -int load_elf(const char *filename,
> - 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 big_endian,
> - int elf_machine, int clear_lsb, int data_swab);
> +ssize_t load_elf(const char *filename,
> + 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 big_endian,
> + int elf_machine, int clear_lsb, int data_swab);
>
> /** load_elf_hdr:
> * @filename: Path of ELF file
> * @hdr: Buffer to populate with header data. Header data will not be
> * filled if set to NULL.
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index c623318b73..c7f97fdce8 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -324,11 +324,11 @@ static void *load_at(int fd, off_t offset, size_t size)
> #define elf_sword int64_t
> #define bswapSZs bswap64s
> #define SZ 64
> #include "hw/elf_ops.h"
>
> -const char *load_elf_strerror(int error)
> +const char *load_elf_strerror(ssize_t error)
> {
> switch (error) {
> case 0:
> return "No error";
> case ELF_LOAD_FAILED:
> @@ -400,62 +400,64 @@ void load_elf_hdr(const char *filename, void *hdr, bool *is64, Error **errp)
> fail:
> close(fd);
> }
>
> /* return < 0 if error, otherwise the number of bytes loaded in memory */
> -int load_elf(const char *filename,
> - 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 big_endian,
> - int elf_machine, int clear_lsb, int data_swab)
> +ssize_t load_elf(const char *filename,
> + 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 big_endian,
> + int elf_machine, int clear_lsb, int data_swab)
> {
> return load_elf_as(filename, elf_note_fn, translate_fn, translate_opaque,
> pentry, lowaddr, highaddr, pflags, big_endian,
> elf_machine, clear_lsb, data_swab, NULL);
> }
>
> /* return < 0 if error, otherwise the number of bytes loaded in memory */
> -int load_elf_as(const char *filename,
> - 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 big_endian,
> - int elf_machine, int clear_lsb, int data_swab, AddressSpace *as)
> +ssize_t load_elf_as(const char *filename,
> + 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 big_endian,
> + int elf_machine, int clear_lsb, int data_swab,
> + AddressSpace *as)
> {
> return load_elf_ram(filename, elf_note_fn, translate_fn, translate_opaque,
> pentry, lowaddr, highaddr, pflags, big_endian,
> elf_machine, clear_lsb, data_swab, as, true);
> }
>
> /* return < 0 if error, otherwise the number of bytes loaded in memory */
> -int load_elf_ram(const char *filename,
> - 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 big_endian,
> - int elf_machine, int clear_lsb, int data_swab,
> - AddressSpace *as, bool load_rom)
> +ssize_t load_elf_ram(const char *filename,
> + 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 big_endian, int elf_machine, int clear_lsb,
> + int data_swab, AddressSpace *as, bool load_rom)
> {
> return load_elf_ram_sym(filename, elf_note_fn,
> translate_fn, translate_opaque,
> pentry, lowaddr, highaddr, pflags, big_endian,
> elf_machine, clear_lsb, data_swab, as,
> load_rom, NULL);
> }
>
> /* return < 0 if error, otherwise the number of bytes loaded in memory */
> -int load_elf_ram_sym(const char *filename,
> - 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 big_endian, int elf_machine,
> - int clear_lsb, int data_swab,
> - AddressSpace *as, bool load_rom, symbol_fn_t sym_cb)
> +ssize_t load_elf_ram_sym(const char *filename,
> + 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 big_endian, int elf_machine,
> + int clear_lsb, int data_swab,
> + AddressSpace *as, bool load_rom, symbol_fn_t sym_cb)
> {
> - int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED;
> + int fd, data_order, target_data_order, must_swab;
> + ssize_t ret = ELF_LOAD_FAILED;
> uint8_t e_ident[EI_NIDENT];
>
> fd = open(filename, O_RDONLY | O_BINARY);
> if (fd < 0) {
> perror(filename);
> --
> 2.17.1
>
--
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/elf_ops.h: switch to ssize_t for elf loader return type
2021-10-06 19:28 [PATCH] hw/elf_ops.h: switch to ssize_t for elf loader return type Luc Michel
2021-10-06 21:09 ` Philippe Mathieu-Daudé
2021-10-12 15:08 ` Luc Michel
@ 2021-10-14 8:36 ` Stefano Garzarella
2021-10-14 8:52 ` Luc Michel
2 siblings, 1 reply; 5+ messages in thread
From: Stefano Garzarella @ 2021-10-14 8:36 UTC (permalink / raw)
To: Luc Michel
Cc: Peter Maydell, Philippe Mathieu-Daudé, qemu-devel, Paolo Bonzini
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?
The rest LGTM.
Thanks,
Stefano
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/elf_ops.h: switch to ssize_t for elf loader return type
2021-10-14 8:36 ` Stefano Garzarella
@ 2021-10-14 8:52 ` Luc Michel
0 siblings, 0 replies; 5+ messages in thread
From: Luc Michel @ 2021-10-14 8:52 UTC (permalink / raw)
To: Stefano Garzarella
Cc: qemu-devel, Peter Maydell, Paolo Bonzini, Philippe Mathieu-Daudé
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
>
--
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-14 8:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-06 19:28 [PATCH] hw/elf_ops.h: switch to ssize_t for elf loader return type Luc Michel
2021-10-06 21:09 ` Philippe Mathieu-Daudé
2021-10-12 15:08 ` Luc Michel
2021-10-14 8:36 ` Stefano Garzarella
2021-10-14 8:52 ` Luc Michel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.