[PULL v2 07/19] elf_ops: Don't try to g_mapped_file_unref(NULL)

Laurent Vivier posted 19 patches 5 years, 6 months ago
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Peter Lieven <pl@kamp.de>, Peter Maydell <peter.maydell@linaro.org>, Kevin Wolf <kwolf@redhat.com>, Jeff Cody <codyprime@gmail.com>, Andrzej Zaborowski <balrogg@gmail.com>, Fam Zheng <fam@euphon.net>, "Michael S. Tsirkin" <mst@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Max Reitz <mreitz@redhat.com>, Juan Quintela <quintela@redhat.com>
There is a newer version of this series
[PULL v2 07/19] elf_ops: Don't try to g_mapped_file_unref(NULL)
Posted by Laurent Vivier 5 years, 6 months ago
From: Peter Maydell <peter.maydell@linaro.org>

Calling g_mapped_file_unref() on a NULL pointer is not valid, and
glib will assert if you try it.

$ qemu-system-arm -M virt -display none -device loader,file=/tmp/bad.elf
qemu-system-arm: -device loader,file=/tmp/bad.elf: GLib: g_mapped_file_unref: assertion 'file != NULL' failed

(One way to produce an ELF file that fails like this is to copy just
the first 16 bytes of a valid ELF file; this is sufficient to fool
the code in load_elf_ram_sym() into thinking it's an ELF file and
calling load_elf32() or load_elf64().)

The failure-exit path in load_elf can be reached from various points
in execution, and for some of those we haven't yet called
g_mapped_file_new_from_fd().  Add a condition to the unref call so we
only call it if we successfully created the GMappedFile to start with.

This will fix the assertion; for the specific case of the generic
loader it will then fall back from "guess this is an ELF file" to
"maybe it's a uImage or a hex file" and eventually to "just load as
a raw data file".

Reported-by: Randy Yates <yates@ieee.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20200423202011.32686-1-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 include/hw/elf_ops.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index e0bb47bb678d..398a4a2c85bb 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -606,7 +606,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
         *highaddr = (uint64_t)(elf_sword)high;
     ret = total_size;
  fail:
-    g_mapped_file_unref(mapped_file);
+    if (mapped_file) {
+        g_mapped_file_unref(mapped_file);
+    }
     g_free(phdr);
     return ret;
 }
-- 
2.26.2