Pablo Greco e6a3ae
From 6dd1f60e226cb3c8ad0791dd6c8edba2493145cd Mon Sep 17 00:00:00 2001
Pablo Greco e6a3ae
From: Cornelia Huck <cohuck@redhat.com>
Pablo Greco e6a3ae
Date: Wed, 17 Apr 2019 13:57:19 +0100
Pablo Greco e6a3ae
Subject: [PATCH 02/24] loader: Check access size when calling rom_ptr() to
Pablo Greco e6a3ae
 avoid crashes
Pablo Greco e6a3ae
Pablo Greco e6a3ae
RH-Author: Cornelia Huck <cohuck@redhat.com>
Pablo Greco e6a3ae
Message-id: <20190417135741.25297-3-cohuck@redhat.com>
Pablo Greco e6a3ae
Patchwork-id: 85785
Pablo Greco e6a3ae
O-Subject: [RHEL-8.1.0 qemu-kvm PATCH v2 02/24] loader: Check access size when calling rom_ptr() to avoid crashes
Pablo Greco e6a3ae
Bugzilla: 1699070
Pablo Greco e6a3ae
RH-Acked-by: David Hildenbrand <david@redhat.com>
Pablo Greco e6a3ae
RH-Acked-by: Thomas Huth <thuth@redhat.com>
Pablo Greco e6a3ae
RH-Acked-by: Jens Freimann <jfreimann@redhat.com>
Pablo Greco e6a3ae
Pablo Greco e6a3ae
From: Thomas Huth <thuth@redhat.com>
Pablo Greco e6a3ae
Pablo Greco e6a3ae
The rom_ptr() function allows direct access to the ROM blobs that we
Pablo Greco e6a3ae
load during startup. However, there are currently no checks for the
Pablo Greco e6a3ae
size of the accesses, so it's currently possible to crash QEMU for
Pablo Greco e6a3ae
example with:
Pablo Greco e6a3ae
Pablo Greco e6a3ae
$ echo "Insane in the mainframe" > /tmp/test.txt
Pablo Greco e6a3ae
$ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz
Pablo Greco e6a3ae
Segmentation fault (core dumped)
Pablo Greco e6a3ae
$ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.txt
Pablo Greco e6a3ae
Segmentation fault (core dumped)
Pablo Greco e6a3ae
$ echo -n HdrS > /tmp/hdr.txt
Pablo Greco e6a3ae
$ sparc64-softmmu/qemu-system-sparc64 -kernel /tmp/hdr.txt -initrd /tmp/hdr.txt
Pablo Greco e6a3ae
Segmentation fault (core dumped)
Pablo Greco e6a3ae
Pablo Greco e6a3ae
We need a possibility to check the size of the ROM area that we want
Pablo Greco e6a3ae
to access, thus let's add a size parameter to the rom_ptr() function
Pablo Greco e6a3ae
to avoid these problems.
Pablo Greco e6a3ae
Pablo Greco e6a3ae
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Pablo Greco e6a3ae
Signed-off-by: Thomas Huth <thuth@redhat.com>
Pablo Greco e6a3ae
Message-Id: <1530005740-25254-1-git-send-email-thuth@redhat.com>
Pablo Greco e6a3ae
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Pablo Greco e6a3ae
(cherry picked from commit 0f0f8b611eeea663c8d3b6021918033e257411a1)
Pablo Greco e6a3ae
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Pablo Greco e6a3ae
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
Pablo Greco e6a3ae
---
Pablo Greco e6a3ae
 hw/core/loader.c     | 10 +++++-----
Pablo Greco e6a3ae
 hw/mips/mips_malta.c |  6 ++++--
Pablo Greco e6a3ae
 hw/s390x/ipl.c       | 18 ++++++++++++------
Pablo Greco e6a3ae
 hw/sparc/sun4m.c     |  4 ++--
Pablo Greco e6a3ae
 hw/sparc64/sun4u.c   |  4 ++--
Pablo Greco e6a3ae
 include/hw/loader.h  |  2 +-
Pablo Greco e6a3ae
 target/arm/cpu.c     |  2 +-
Pablo Greco e6a3ae
 7 files changed, 27 insertions(+), 19 deletions(-)
Pablo Greco e6a3ae
Pablo Greco e6a3ae
diff --git a/hw/core/loader.c b/hw/core/loader.c
Pablo Greco e6a3ae
index 06bdbca..bbb6e65 100644
Pablo Greco e6a3ae
--- a/hw/core/loader.c
Pablo Greco e6a3ae
+++ b/hw/core/loader.c
Pablo Greco e6a3ae
@@ -191,7 +191,7 @@ void pstrcpy_targphys(const char *name, hwaddr dest, int buf_size,
Pablo Greco e6a3ae
         rom_add_blob_fixed(name, source, (nulp - source) + 1, dest);
Pablo Greco e6a3ae
     } else {
Pablo Greco e6a3ae
         rom_add_blob_fixed(name, source, buf_size, dest);
Pablo Greco e6a3ae
-        ptr = rom_ptr(dest + buf_size - 1);
Pablo Greco e6a3ae
+        ptr = rom_ptr(dest + buf_size - 1, sizeof(*ptr));
Pablo Greco e6a3ae
         *ptr = 0;
Pablo Greco e6a3ae
     }
Pablo Greco e6a3ae
 }
Pablo Greco e6a3ae
@@ -1165,7 +1165,7 @@ void rom_reset_order_override(void)
Pablo Greco e6a3ae
     fw_cfg_reset_order_override(fw_cfg);
Pablo Greco e6a3ae
 }
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
-static Rom *find_rom(hwaddr addr)
Pablo Greco e6a3ae
+static Rom *find_rom(hwaddr addr, size_t size)
Pablo Greco e6a3ae
 {
Pablo Greco e6a3ae
     Rom *rom;
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
@@ -1179,7 +1179,7 @@ static Rom *find_rom(hwaddr addr)
Pablo Greco e6a3ae
         if (rom->addr > addr) {
Pablo Greco e6a3ae
             continue;
Pablo Greco e6a3ae
         }
Pablo Greco e6a3ae
-        if (rom->addr + rom->romsize < addr) {
Pablo Greco e6a3ae
+        if (rom->addr + rom->romsize < addr + size) {
Pablo Greco e6a3ae
             continue;
Pablo Greco e6a3ae
         }
Pablo Greco e6a3ae
         return rom;
Pablo Greco e6a3ae
@@ -1249,11 +1249,11 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size)
Pablo Greco e6a3ae
     return (d + l) - dest;
Pablo Greco e6a3ae
 }
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
-void *rom_ptr(hwaddr addr)
Pablo Greco e6a3ae
+void *rom_ptr(hwaddr addr, size_t size)
Pablo Greco e6a3ae
 {
Pablo Greco e6a3ae
     Rom *rom;
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
-    rom = find_rom(addr);
Pablo Greco e6a3ae
+    rom = find_rom(addr, size);
Pablo Greco e6a3ae
     if (!rom || !rom->data)
Pablo Greco e6a3ae
         return NULL;
Pablo Greco e6a3ae
     return rom->data + (addr - rom->addr);
Pablo Greco e6a3ae
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
Pablo Greco e6a3ae
index f6513a4..8bbea0b 100644
Pablo Greco e6a3ae
--- a/hw/mips/mips_malta.c
Pablo Greco e6a3ae
+++ b/hw/mips/mips_malta.c
Pablo Greco e6a3ae
@@ -1139,11 +1139,13 @@ void mips_malta_init(MachineState *machine)
Pablo Greco e6a3ae
            a neat trick which allows bi-endian firmware. */
Pablo Greco e6a3ae
 #ifndef TARGET_WORDS_BIGENDIAN
Pablo Greco e6a3ae
         {
Pablo Greco e6a3ae
-            uint32_t *end, *addr = rom_ptr(FLASH_ADDRESS);
Pablo Greco e6a3ae
+            uint32_t *end, *addr;
Pablo Greco e6a3ae
+            const size_t swapsize = MIN(bios_size, 0x3e0000);
Pablo Greco e6a3ae
+            addr = rom_ptr(FLASH_ADDRESS, swapsize);
Pablo Greco e6a3ae
             if (!addr) {
Pablo Greco e6a3ae
                 addr = memory_region_get_ram_ptr(bios);
Pablo Greco e6a3ae
             }
Pablo Greco e6a3ae
-            end = (void *)addr + MIN(bios_size, 0x3e0000);
Pablo Greco e6a3ae
+            end = (void *)addr + swapsize;
Pablo Greco e6a3ae
             while (addr < end) {
Pablo Greco e6a3ae
                 bswap32s(addr);
Pablo Greco e6a3ae
                 addr++;
Pablo Greco e6a3ae
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
Pablo Greco e6a3ae
index 617ac43..10038ec 100644
Pablo Greco e6a3ae
--- a/hw/s390x/ipl.c
Pablo Greco e6a3ae
+++ b/hw/s390x/ipl.c
Pablo Greco e6a3ae
@@ -32,7 +32,6 @@
Pablo Greco e6a3ae
 #define KERN_PARM_AREA                  0x010480UL
Pablo Greco e6a3ae
 #define INITRD_START                    0x800000UL
Pablo Greco e6a3ae
 #define INITRD_PARM_START               0x010408UL
Pablo Greco e6a3ae
-#define INITRD_PARM_SIZE                0x010410UL
Pablo Greco e6a3ae
 #define PARMFILE_START                  0x001000UL
Pablo Greco e6a3ae
 #define ZIPL_IMAGE_START                0x009000UL
Pablo Greco e6a3ae
 #define IPL_PSW_MASK                    (PSW_MASK_32 | PSW_MASK_64)
Pablo Greco e6a3ae
@@ -164,12 +163,12 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
Pablo Greco e6a3ae
                 goto error;
Pablo Greco e6a3ae
             }
Pablo Greco e6a3ae
             /* if this is Linux use KERN_IMAGE_START */
Pablo Greco e6a3ae
-            magic = rom_ptr(LINUX_MAGIC_ADDR);
Pablo Greco e6a3ae
+            magic = rom_ptr(LINUX_MAGIC_ADDR, 6);
Pablo Greco e6a3ae
             if (magic && !memcmp(magic, "S390EP", 6)) {
Pablo Greco e6a3ae
                 pentry = KERN_IMAGE_START;
Pablo Greco e6a3ae
             } else {
Pablo Greco e6a3ae
                 /* if not Linux load the address of the (short) IPL PSW */
Pablo Greco e6a3ae
-                ipl_psw = rom_ptr(4);
Pablo Greco e6a3ae
+                ipl_psw = rom_ptr(4, 4);
Pablo Greco e6a3ae
                 if (ipl_psw) {
Pablo Greco e6a3ae
                     pentry = be32_to_cpu(*ipl_psw) & 0x7fffffffUL;
Pablo Greco e6a3ae
                 } else {
Pablo Greco e6a3ae
@@ -185,9 +184,12 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
Pablo Greco e6a3ae
          * loader) and it won't work. For this case we force it to 0x10000, too.
Pablo Greco e6a3ae
          */
Pablo Greco e6a3ae
         if (pentry == KERN_IMAGE_START || pentry == 0x800) {
Pablo Greco e6a3ae
+            char *parm_area = rom_ptr(KERN_PARM_AREA, strlen(ipl->cmdline) + 1);
Pablo Greco e6a3ae
             ipl->start_addr = KERN_IMAGE_START;
Pablo Greco e6a3ae
             /* Overwrite parameters in the kernel image, which are "rom" */
Pablo Greco e6a3ae
-            strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
Pablo Greco e6a3ae
+            if (parm_area) {
Pablo Greco e6a3ae
+                strcpy(parm_area, ipl->cmdline);
Pablo Greco e6a3ae
+            }
Pablo Greco e6a3ae
         } else {
Pablo Greco e6a3ae
             ipl->start_addr = pentry;
Pablo Greco e6a3ae
         }
Pablo Greco e6a3ae
@@ -195,6 +197,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
Pablo Greco e6a3ae
         if (ipl->initrd) {
Pablo Greco e6a3ae
             ram_addr_t initrd_offset;
Pablo Greco e6a3ae
             int initrd_size;
Pablo Greco e6a3ae
+            uint64_t *romptr;
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
             initrd_offset = INITRD_START;
Pablo Greco e6a3ae
             while (kernel_size + 0x100000 > initrd_offset) {
Pablo Greco e6a3ae
@@ -211,8 +214,11 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
Pablo Greco e6a3ae
              * we have to overwrite values in the kernel image,
Pablo Greco e6a3ae
              * which are "rom"
Pablo Greco e6a3ae
              */
Pablo Greco e6a3ae
-            stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
Pablo Greco e6a3ae
-            stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
Pablo Greco e6a3ae
+            romptr = rom_ptr(INITRD_PARM_START, 16);
Pablo Greco e6a3ae
+            if (romptr) {
Pablo Greco e6a3ae
+                stq_p(romptr, initrd_offset);
Pablo Greco e6a3ae
+                stq_p(romptr + 1, initrd_size);
Pablo Greco e6a3ae
+            }
Pablo Greco e6a3ae
         }
Pablo Greco e6a3ae
     }
Pablo Greco e6a3ae
     /*
Pablo Greco e6a3ae
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
Pablo Greco e6a3ae
index 6471aca..a400442 100644
Pablo Greco e6a3ae
--- a/hw/sparc/sun4m.c
Pablo Greco e6a3ae
+++ b/hw/sparc/sun4m.c
Pablo Greco e6a3ae
@@ -272,8 +272,8 @@ static unsigned long sun4m_load_kernel(const char *kernel_filename,
Pablo Greco e6a3ae
         }
Pablo Greco e6a3ae
         if (initrd_size > 0) {
Pablo Greco e6a3ae
             for (i = 0; i < 64 * TARGET_PAGE_SIZE; i += TARGET_PAGE_SIZE) {
Pablo Greco e6a3ae
-                ptr = rom_ptr(KERNEL_LOAD_ADDR + i);
Pablo Greco e6a3ae
-                if (ldl_p(ptr) == 0x48647253) { // HdrS
Pablo Greco e6a3ae
+                ptr = rom_ptr(KERNEL_LOAD_ADDR + i, 24);
Pablo Greco e6a3ae
+                if (ptr && ldl_p(ptr) == 0x48647253) { /* HdrS */
Pablo Greco e6a3ae
                     stl_p(ptr + 16, INITRD_LOAD_ADDR);
Pablo Greco e6a3ae
                     stl_p(ptr + 20, initrd_size);
Pablo Greco e6a3ae
                     break;
Pablo Greco e6a3ae
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
Pablo Greco e6a3ae
index 2044a52..1410c3a 100644
Pablo Greco e6a3ae
--- a/hw/sparc64/sun4u.c
Pablo Greco e6a3ae
+++ b/hw/sparc64/sun4u.c
Pablo Greco e6a3ae
@@ -186,8 +186,8 @@ static uint64_t sun4u_load_kernel(const char *kernel_filename,
Pablo Greco e6a3ae
         }
Pablo Greco e6a3ae
         if (*initrd_size > 0) {
Pablo Greco e6a3ae
             for (i = 0; i < 64 * TARGET_PAGE_SIZE; i += TARGET_PAGE_SIZE) {
Pablo Greco e6a3ae
-                ptr = rom_ptr(*kernel_addr + i);
Pablo Greco e6a3ae
-                if (ldl_p(ptr + 8) == 0x48647253) { /* HdrS */
Pablo Greco e6a3ae
+                ptr = rom_ptr(*kernel_addr + i, 32);
Pablo Greco e6a3ae
+                if (ptr && ldl_p(ptr + 8) == 0x48647253) { /* HdrS */
Pablo Greco e6a3ae
                     stl_p(ptr + 24, *initrd_addr + *kernel_addr);
Pablo Greco e6a3ae
                     stl_p(ptr + 28, *initrd_size);
Pablo Greco e6a3ae
                     break;
Pablo Greco e6a3ae
diff --git a/include/hw/loader.h b/include/hw/loader.h
Pablo Greco e6a3ae
index 5ed3fd8..e98b84b 100644
Pablo Greco e6a3ae
--- a/include/hw/loader.h
Pablo Greco e6a3ae
+++ b/include/hw/loader.h
Pablo Greco e6a3ae
@@ -226,7 +226,7 @@ void rom_set_fw(FWCfgState *f);
Pablo Greco e6a3ae
 void rom_set_order_override(int order);
Pablo Greco e6a3ae
 void rom_reset_order_override(void);
Pablo Greco e6a3ae
 int rom_copy(uint8_t *dest, hwaddr addr, size_t size);
Pablo Greco e6a3ae
-void *rom_ptr(hwaddr addr);
Pablo Greco e6a3ae
+void *rom_ptr(hwaddr addr, size_t size);
Pablo Greco e6a3ae
 void hmp_info_roms(Monitor *mon, const QDict *qdict);
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
 #define rom_add_file_fixed(_f, _a, _i)          \
Pablo Greco e6a3ae
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
Pablo Greco e6a3ae
index 9d030e0..6773f40 100644
Pablo Greco e6a3ae
--- a/target/arm/cpu.c
Pablo Greco e6a3ae
+++ b/target/arm/cpu.c
Pablo Greco e6a3ae
@@ -219,7 +219,7 @@ static void arm_cpu_reset(CPUState *s)
Pablo Greco e6a3ae
 
Pablo Greco e6a3ae
         /* Load the initial SP and PC from offset 0 and 4 in the vector table */
Pablo Greco e6a3ae
         vecbase = env->v7m.vecbase[env->v7m.secure];
Pablo Greco e6a3ae
-        rom = rom_ptr(vecbase);
Pablo Greco e6a3ae
+        rom = rom_ptr(vecbase, 8);
Pablo Greco e6a3ae
         if (rom) {
Pablo Greco e6a3ae
             /* Address zero is covered by ROM which hasn't yet been
Pablo Greco e6a3ae
              * copied into physical memory.
Pablo Greco e6a3ae
-- 
Pablo Greco e6a3ae
1.8.3.1
Pablo Greco e6a3ae