From nobody Tue Nov 4 18:51:48 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1530530588941696.50942425172; Mon, 2 Jul 2018 04:23:08 -0700 (PDT) Received: from localhost ([::1]:60112 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fZwvD-0005ev-Vn for importer@patchew.org; Mon, 02 Jul 2018 07:23:08 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43633) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fZwq9-0001vY-4y for qemu-devel@nongnu.org; Mon, 02 Jul 2018 07:17:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fZwq7-0005zq-O7 for qemu-devel@nongnu.org; Mon, 02 Jul 2018 07:17:53 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:33460 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fZwq7-0005zP-IX; Mon, 02 Jul 2018 07:17:51 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F0F1B87AC7; Mon, 2 Jul 2018 11:17:50 +0000 (UTC) Received: from localhost (ovpn-116-251.ams2.redhat.com [10.36.116.251]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 752862166B5D; Mon, 2 Jul 2018 11:17:50 +0000 (UTC) From: Cornelia Huck To: Peter Maydell Date: Mon, 2 Jul 2018 13:17:24 +0200 Message-Id: <20180702111737.18970-3-cohuck@redhat.com> In-Reply-To: <20180702111737.18970-1-cohuck@redhat.com> References: <20180702111737.18970-1-cohuck@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 02 Jul 2018 11:17:51 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 02 Jul 2018 11:17:51 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'cohuck@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 02/15] loader: Check access size when calling rom_ptr() to avoid crashes X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Thomas Huth , David Hildenbrand , Cornelia Huck , Alexander Graf , qemu-devel@nongnu.org, Christian Borntraeger , qemu-s390x@nongnu.org, Richard Henderson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Thomas Huth The rom_ptr() function allows direct access to the ROM blobs that we load during startup. However, there are currently no checks for the size of the accesses, so it's currently possible to crash QEMU for example with: $ echo "Insane in the mainframe" > /tmp/test.txt $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -append xyz Segmentation fault (core dumped) $ s390x-softmmu/qemu-system-s390x -kernel /tmp/test.txt -initrd /tmp/test.t= xt Segmentation fault (core dumped) $ echo -n HdrS > /tmp/hdr.txt $ sparc64-softmmu/qemu-system-sparc64 -kernel /tmp/hdr.txt -initrd /tmp/hdr= .txt Segmentation fault (core dumped) We need a possibility to check the size of the ROM area that we want to access, thus let's add a size parameter to the rom_ptr() function to avoid these problems. Acked-by: Christian Borntraeger Signed-off-by: Thomas Huth Message-Id: <1530005740-25254-1-git-send-email-thuth@redhat.com> Signed-off-by: Cornelia Huck --- hw/core/loader.c | 10 +++++----- hw/mips/mips_malta.c | 6 ++++-- hw/s390x/ipl.c | 18 ++++++++++++------ hw/sparc/sun4m.c | 4 ++-- hw/sparc64/sun4u.c | 4 ++-- include/hw/loader.h | 2 +- target/arm/cpu.c | 2 +- 7 files changed, 27 insertions(+), 19 deletions(-) diff --git a/hw/core/loader.c b/hw/core/loader.c index 06bdbca537..bbb6e65bb5 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -191,7 +191,7 @@ void pstrcpy_targphys(const char *name, hwaddr dest, in= t buf_size, rom_add_blob_fixed(name, source, (nulp - source) + 1, dest); } else { rom_add_blob_fixed(name, source, buf_size, dest); - ptr =3D rom_ptr(dest + buf_size - 1); + ptr =3D rom_ptr(dest + buf_size - 1, sizeof(*ptr)); *ptr =3D 0; } } @@ -1165,7 +1165,7 @@ void rom_reset_order_override(void) fw_cfg_reset_order_override(fw_cfg); } =20 -static Rom *find_rom(hwaddr addr) +static Rom *find_rom(hwaddr addr, size_t size) { Rom *rom; =20 @@ -1179,7 +1179,7 @@ static Rom *find_rom(hwaddr addr) if (rom->addr > addr) { continue; } - if (rom->addr + rom->romsize < addr) { + if (rom->addr + rom->romsize < addr + size) { continue; } return rom; @@ -1249,11 +1249,11 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t siz= e) return (d + l) - dest; } =20 -void *rom_ptr(hwaddr addr) +void *rom_ptr(hwaddr addr, size_t size) { Rom *rom; =20 - rom =3D find_rom(addr); + rom =3D find_rom(addr, size); if (!rom || !rom->data) return NULL; return rom->data + (addr - rom->addr); diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index b9d92bf47e..1b4e32e58e 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -1133,11 +1133,13 @@ void mips_malta_init(MachineState *machine) a neat trick which allows bi-endian firmware. */ #ifndef TARGET_WORDS_BIGENDIAN { - uint32_t *end, *addr =3D rom_ptr(FLASH_ADDRESS); + uint32_t *end, *addr; + const size_t swapsize =3D MIN(bios_size, 0x3e0000); + addr =3D rom_ptr(FLASH_ADDRESS, swapsize); if (!addr) { addr =3D memory_region_get_ram_ptr(bios); } - end =3D (void *)addr + MIN(bios_size, 0x3e0000); + end =3D (void *)addr + swapsize; while (addr < end) { bswap32s(addr); addr++; diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index f278036fa7..21f64ad26a 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -33,7 +33,6 @@ #define KERN_PARM_AREA 0x010480UL #define INITRD_START 0x800000UL #define INITRD_PARM_START 0x010408UL -#define INITRD_PARM_SIZE 0x010410UL #define PARMFILE_START 0x001000UL #define ZIPL_IMAGE_START 0x009000UL #define IPL_PSW_MASK (PSW_MASK_32 | PSW_MASK_64) @@ -165,12 +164,12 @@ static void s390_ipl_realize(DeviceState *dev, Error = **errp) goto error; } /* if this is Linux use KERN_IMAGE_START */ - magic =3D rom_ptr(LINUX_MAGIC_ADDR); + magic =3D rom_ptr(LINUX_MAGIC_ADDR, 6); if (magic && !memcmp(magic, "S390EP", 6)) { pentry =3D KERN_IMAGE_START; } else { /* if not Linux load the address of the (short) IPL PSW */ - ipl_psw =3D rom_ptr(4); + ipl_psw =3D rom_ptr(4, 4); if (ipl_psw) { pentry =3D be32_to_cpu(*ipl_psw) & 0x7fffffffUL; } else { @@ -186,9 +185,12 @@ static void s390_ipl_realize(DeviceState *dev, Error *= *errp) * loader) and it won't work. For this case we force it to 0x10000= , too. */ if (pentry =3D=3D KERN_IMAGE_START || pentry =3D=3D 0x800) { + char *parm_area =3D rom_ptr(KERN_PARM_AREA, strlen(ipl->cmdlin= e) + 1); ipl->start_addr =3D KERN_IMAGE_START; /* Overwrite parameters in the kernel image, which are "rom" */ - strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline); + if (parm_area) { + strcpy(parm_area, ipl->cmdline); + } } else { ipl->start_addr =3D pentry; } @@ -196,6 +198,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **= errp) if (ipl->initrd) { ram_addr_t initrd_offset; int initrd_size; + uint64_t *romptr; =20 initrd_offset =3D INITRD_START; while (kernel_size + 0x100000 > initrd_offset) { @@ -212,8 +215,11 @@ static void s390_ipl_realize(DeviceState *dev, Error *= *errp) * we have to overwrite values in the kernel image, * which are "rom" */ - stq_p(rom_ptr(INITRD_PARM_START), initrd_offset); - stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size); + romptr =3D rom_ptr(INITRD_PARM_START, 16); + if (romptr) { + stq_p(romptr, initrd_offset); + stq_p(romptr + 1, initrd_size); + } } } /* diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c index b984d2da0e..21078cc121 100644 --- a/hw/sparc/sun4m.c +++ b/hw/sparc/sun4m.c @@ -272,8 +272,8 @@ static unsigned long sun4m_load_kernel(const char *kern= el_filename, } if (initrd_size > 0) { for (i =3D 0; i < 64 * TARGET_PAGE_SIZE; i +=3D TARGET_PAGE_SI= ZE) { - ptr =3D rom_ptr(KERNEL_LOAD_ADDR + i); - if (ldl_p(ptr) =3D=3D 0x48647253) { // HdrS + ptr =3D rom_ptr(KERNEL_LOAD_ADDR + i, 24); + if (ptr && ldl_p(ptr) =3D=3D 0x48647253) { /* HdrS */ stl_p(ptr + 16, INITRD_LOAD_ADDR); stl_p(ptr + 20, initrd_size); break; diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c index 3975a7b65a..334dd7008e 100644 --- a/hw/sparc64/sun4u.c +++ b/hw/sparc64/sun4u.c @@ -186,8 +186,8 @@ static uint64_t sun4u_load_kernel(const char *kernel_fi= lename, } if (*initrd_size > 0) { for (i =3D 0; i < 64 * TARGET_PAGE_SIZE; i +=3D TARGET_PAGE_SI= ZE) { - ptr =3D rom_ptr(*kernel_addr + i); - if (ldl_p(ptr + 8) =3D=3D 0x48647253) { /* HdrS */ + ptr =3D rom_ptr(*kernel_addr + i, 32); + if (ptr && ldl_p(ptr + 8) =3D=3D 0x48647253) { /* HdrS */ stl_p(ptr + 24, *initrd_addr + *kernel_addr); stl_p(ptr + 28, *initrd_size); break; diff --git a/include/hw/loader.h b/include/hw/loader.h index 5ed3fd8ae6..e98b84b8f9 100644 --- a/include/hw/loader.h +++ b/include/hw/loader.h @@ -226,7 +226,7 @@ void rom_set_fw(FWCfgState *f); void rom_set_order_override(int order); void rom_reset_order_override(void); int rom_copy(uint8_t *dest, hwaddr addr, size_t size); -void *rom_ptr(hwaddr addr); +void *rom_ptr(hwaddr addr, size_t size); void hmp_info_roms(Monitor *mon, const QDict *qdict); =20 #define rom_add_file_fixed(_f, _a, _i) \ diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 82ff450f9a..64a8005a4b 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -239,7 +239,7 @@ static void arm_cpu_reset(CPUState *s) =20 /* Load the initial SP and PC from offset 0 and 4 in the vector ta= ble */ vecbase =3D env->v7m.vecbase[env->v7m.secure]; - rom =3D rom_ptr(vecbase); + rom =3D rom_ptr(vecbase, 8); if (rom) { /* Address zero is covered by ROM which hasn't yet been * copied into physical memory. --=20 2.14.4