[Qemu-devel] [PATCH 1/2] arm: fix load ELF error leak

[Qemu-devel] [PATCH 1/2] arm: fix load ELF error leak

[Marc-André Lureau]

Spotted by ASAN:
QTEST_QEMU_BINARY=aarch64-softmmu/qemu-system-aarch64 tests/boot-serial-test

Direct leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x7ff8a9b0ca38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
    #1 0x7ff8a8ea7f75 in g_malloc0 ../glib/gmem.c:124
    #2 0x55fef3d99129 in error_setv /home/elmarco/src/qemu/util/error.c:59
    #3 0x55fef3d99738 in error_setg_internal /home/elmarco/src/qemu/util/error.c:95
    #4 0x55fef323acb2 in load_elf_hdr /home/elmarco/src/qemu/hw/core/loader.c:393
    #5 0x55fef2d15776 in arm_load_elf /home/elmarco/src/qemu/hw/arm/boot.c:830
    #6 0x55fef2d16d39 in arm_load_kernel_notify /home/elmarco/src/qemu/hw/arm/boot.c:1022
    #7 0x55fef3dc634d in notifier_list_notify /home/elmarco/src/qemu/util/notify.c:40
    #8 0x55fef2fc3182 in qemu_run_machine_init_done_notifiers /home/elmarco/src/qemu/vl.c:2716
    #9 0x55fef2fcbbd1 in main /home/elmarco/src/qemu/vl.c:4679
    #10 0x7ff89dfed009 in __libc_start_main (/lib64/libc.so.6+0x21009)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/arm/boot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 6d0c92ab88..784d301683 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -829,6 +829,7 @@ static uint64_t arm_load_elf(struct arm_boot_info *info, uint64_t *pentry,
 
     load_elf_hdr(info->kernel_filename, &elf_header, &elf_is64, &err);
     if (err) {
+        error_free(err);
         return ret;
     }
 
-- 
2.16.2.346.g9779355e34

[Qemu-devel] [PATCH 2/2] arm: avoid heap-buffer-overflow in load_aarch64_image

[Marc-André Lureau]

Spotted by ASAN:

elmarco@boraha:~/src/qemu/build (master *%)$ QTEST_QEMU_BINARY=aarch64-softmmu/qemu-system-aarch64 tests/boot-serial-test
/aarch64/boot-serial/virt: ** (process:19740): DEBUG: 18:39:30.275: foo /tmp/qtest-boot-serial-cXaS94D
=================================================================
==19740==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000069648 at pc 0x7f1d2201cc54 bp 0x7fff331f6a40 sp 0x7fff331f61e8
READ of size 4 at 0x603000069648 thread T0
    #0 0x7f1d2201cc53  (/lib64/libasan.so.4+0xafc53)
    #1 0x55bc86685ee3 in load_aarch64_image /home/elmarco/src/qemu/hw/arm/boot.c:894
    #2 0x55bc86687217 in arm_load_kernel_notify /home/elmarco/src/qemu/hw/arm/boot.c:1047
    #3 0x55bc877363b5 in notifier_list_notify /home/elmarco/src/qemu/util/notify.c:40
    #4 0x55bc869331ea in qemu_run_machine_init_done_notifiers /home/elmarco/src/qemu/vl.c:2716
    #5 0x55bc8693bc39 in main /home/elmarco/src/qemu/vl.c:4679
    #6 0x7f1d1652c009 in __libc_start_main (/lib64/libc.so.6+0x21009)
    #7 0x55bc86255cc9 in _start (/home/elmarco/src/qemu/build/aarch64-softmmu/qemu-system-aarch64+0x1ae5cc9)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/arm/boot.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 784d301683..196c7fb242 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -891,7 +891,8 @@ static uint64_t load_aarch64_image(const char *filename, hwaddr mem_base,
     }
 
     /* check the arm64 magic header value -- very old kernels may not have it */
-    if (memcmp(buffer + ARM64_MAGIC_OFFSET, "ARM\x64", 4) == 0) {
+    if (size > ARM64_MAGIC_OFFSET + 4 &&
+        memcmp(buffer + ARM64_MAGIC_OFFSET, "ARM\x64", 4) == 0) {
         uint64_t hdrvals[2];
 
         /* The arm64 Image header has text_offset and image_size fields at 8 and
-- 
2.16.2.346.g9779355e34

Re: [Qemu-devel] [PATCH 1/2] arm: fix load ELF error leak

[Peter Maydell]

On 7 March 2018 at 18:03, Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> Spotted by ASAN:
> QTEST_QEMU_BINARY=aarch64-softmmu/qemu-system-aarch64 tests/boot-serial-test
>
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>     #0 0x7ff8a9b0ca38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
>     #1 0x7ff8a8ea7f75 in g_malloc0 ../glib/gmem.c:124
>     #2 0x55fef3d99129 in error_setv /home/elmarco/src/qemu/util/error.c:59
>     #3 0x55fef3d99738 in error_setg_internal /home/elmarco/src/qemu/util/error.c:95
>     #4 0x55fef323acb2 in load_elf_hdr /home/elmarco/src/qemu/hw/core/loader.c:393
>     #5 0x55fef2d15776 in arm_load_elf /home/elmarco/src/qemu/hw/arm/boot.c:830
>     #6 0x55fef2d16d39 in arm_load_kernel_notify /home/elmarco/src/qemu/hw/arm/boot.c:1022
>     #7 0x55fef3dc634d in notifier_list_notify /home/elmarco/src/qemu/util/notify.c:40
>     #8 0x55fef2fc3182 in qemu_run_machine_init_done_notifiers /home/elmarco/src/qemu/vl.c:2716
>     #9 0x55fef2fcbbd1 in main /home/elmarco/src/qemu/vl.c:4679
>     #10 0x7ff89dfed009 in __libc_start_main (/lib64/libc.so.6+0x21009)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks, applied patches 1 and 2 to target-arm.next.

For the future, when you're sending a patchset that's more than one
patch can you make sure you send a cover letter, please? The automated
tooling wants to see the cover letter, and if there isn't one I have
to mess about with fishing the patches out of email and applying them
by hand, which is a faff.

thanks
-- PMM