[PATCH v2] hw/elf_ops: defend against weird elf headers

Alex Bennée posted 1 patch 1 week, 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260812081405.3811787-1-alex.bennee@linaro.org
include/hw/elf_ops.h.inc | 5 +++++
1 file changed, 5 insertions(+)
[PATCH v2] hw/elf_ops: defend against weird elf headers
Posted by Alex Bennée 1 week, 6 days ago
According to the ELF spec:

  PT_LOAD

  The array element specifies a loadable segment, described by
  p_filesz and p_memsz. The bytes from the file are mapped to the
  beginning of the memory segment. If the segment's memory
  size (p_memsz) is larger than the file size (p_filesz), the
  ``extra'' bytes are defined to hold the value 0 and to follow the
  segment's initialized area. The file size may not be larger than the
  memory size. Loadable segment entries in the program header table
  appear in ascending order, sorted on the p_vaddr member.

which implies while both p_filesz and p_memsz can be zero we should
never see a case where p_filesz is greater than the in memory size.
Indeed it has been reported such a hand crafted ELF can blow up, for
example during rom_reset():

  address_space_set(rom->as, rom->addr + rom->datasize, 0,
                    rom->romsize - rom->datasize,
                    MEMTXATTRS_UNSPECIFIED);

which could trigger and underflow leaving QEMU slowly filling a very
large buffer.

Fixes: https://gitlab.com/qemu-project/qemu/-/work_items/4056
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: qemu-stable@nongnu.org

---
v2
  - ret = ELF_LOAD_TOO_BIG
  - tweak subject
---
 include/hw/elf_ops.h.inc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/hw/elf_ops.h.inc b/include/hw/elf_ops.h.inc
index 9c35d1b9da6..044e72de2a2 100644
--- a/include/hw/elf_ops.h.inc
+++ b/include/hw/elf_ops.h.inc
@@ -427,6 +427,11 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
             file_size = ph->p_filesz; /* Size of the allocated data */
             data_offset = ph->p_offset; /* Offset where the data is located */
 
+            if (file_size > mem_size) {
+                ret = ELF_LOAD_TOO_BIG;
+                goto fail;
+            }
+
             if (file_size > 0) {
                 if (g_mapped_file_get_length(mapped_file) <
                     file_size + data_offset) {
-- 
2.47.3


Re: [PATCH v2] hw/elf_ops: defend against weird elf headers
Posted by Philippe Mathieu-Daudé 1 week, 3 days ago
On 12/8/26 10:14, Alex Bennée wrote:
> According to the ELF spec:
> 
>    PT_LOAD
> 
>    The array element specifies a loadable segment, described by
>    p_filesz and p_memsz. The bytes from the file are mapped to the
>    beginning of the memory segment. If the segment's memory
>    size (p_memsz) is larger than the file size (p_filesz), the
>    ``extra'' bytes are defined to hold the value 0 and to follow the
>    segment's initialized area. The file size may not be larger than the
>    memory size. Loadable segment entries in the program header table
>    appear in ascending order, sorted on the p_vaddr member.
> 
> which implies while both p_filesz and p_memsz can be zero we should
> never see a case where p_filesz is greater than the in memory size.
> Indeed it has been reported such a hand crafted ELF can blow up, for
> example during rom_reset():
> 
>    address_space_set(rom->as, rom->addr + rom->datasize, 0,
>                      rom->romsize - rom->datasize,
>                      MEMTXATTRS_UNSPECIFIED);
> 
> which could trigger and underflow leaving QEMU slowly filling a very
> large buffer.
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/work_items/4056
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: qemu-stable@nongnu.org
> 
> ---
> v2
>    - ret = ELF_LOAD_TOO_BIG
>    - tweak subject
> ---
>   include/hw/elf_ops.h.inc | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

and queued via hw-misc tree, thanks.

Re: [PATCH v2] hw/elf_ops: defend against weird elf headers
Posted by Richard Henderson 1 week, 6 days ago
On 8/12/26 01:14, Alex Bennée wrote:
> According to the ELF spec:
> 
>    PT_LOAD
> 
>    The array element specifies a loadable segment, described by
>    p_filesz and p_memsz. The bytes from the file are mapped to the
>    beginning of the memory segment. If the segment's memory
>    size (p_memsz) is larger than the file size (p_filesz), the
>    ``extra'' bytes are defined to hold the value 0 and to follow the
>    segment's initialized area. The file size may not be larger than the
>    memory size. Loadable segment entries in the program header table
>    appear in ascending order, sorted on the p_vaddr member.
> 
> which implies while both p_filesz and p_memsz can be zero we should
> never see a case where p_filesz is greater than the in memory size.
> Indeed it has been reported such a hand crafted ELF can blow up, for
> example during rom_reset():
> 
>    address_space_set(rom->as, rom->addr + rom->datasize, 0,
>                      rom->romsize - rom->datasize,
>                      MEMTXATTRS_UNSPECIFIED);
> 
> which could trigger and underflow leaving QEMU slowly filling a very
> large buffer.
> 
> Fixes:https://gitlab.com/qemu-project/qemu/-/work_items/4056
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> Cc:qemu-stable@nongnu.org
> 
> ---
> v2
>    - ret = ELF_LOAD_TOO_BIG
>    - tweak subject
> ---
>   include/hw/elf_ops.h.inc | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~