hw/core/loader.c | 10 +++++----- hw/mips/mips_malta.c | 6 ++++-- hw/s390x/ipl.c | 13 ++++++++++--- hw/sparc/sun4m.c | 2 +- hw/sparc64/sun4u.c | 2 +- include/hw/loader.h | 2 +- target/arm/cpu.c | 2 +- 7 files changed, 23 insertions(+), 14 deletions(-)
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.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.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/core/loader.c | 10 +++++-----
hw/mips/mips_malta.c | 6 ++++--
hw/s390x/ipl.c | 13 ++++++++++---
hw/sparc/sun4m.c | 2 +-
hw/sparc64/sun4u.c | 2 +-
include/hw/loader.h | 2 +-
target/arm/cpu.c | 2 +-
7 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 06bdbca..54be522 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -191,7 +191,7 @@ void pstrcpy_targphys(const char *name, hwaddr dest, int buf_size,
rom_add_blob_fixed(name, source, (nulp - source) + 1, dest);
} else {
rom_add_blob_fixed(name, source, buf_size, dest);
- ptr = rom_ptr(dest + buf_size - 1);
+ ptr = rom_ptr(dest + buf_size - 1, sizeof(*ptr));
*ptr = 0;
}
}
@@ -1165,7 +1165,7 @@ void rom_reset_order_override(void)
fw_cfg_reset_order_override(fw_cfg);
}
-static Rom *find_rom(hwaddr addr)
+static Rom *find_rom(hwaddr addr, size_t size)
{
Rom *rom;
@@ -1176,7 +1176,7 @@ static Rom *find_rom(hwaddr addr)
if (rom->mr) {
continue;
}
- if (rom->addr > addr) {
+ if (rom->addr > addr + size) {
continue;
}
if (rom->addr + rom->romsize < addr) {
@@ -1249,11 +1249,11 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size)
return (d + l) - dest;
}
-void *rom_ptr(hwaddr addr)
+void *rom_ptr(hwaddr addr, size_t size)
{
Rom *rom;
- rom = find_rom(addr);
+ rom = 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 494f84e..3733e2f 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 = rom_ptr(FLASH_ADDRESS);
+ uint32_t *end, *addr;
+ const size_t swapsize = MIN(bios_size, 0x3e0000);
+ addr = rom_ptr(FLASH_ADDRESS, swapsize);
if (!addr) {
addr = memory_region_get_ram_ptr(bios);
}
- end = (void *)addr + MIN(bios_size, 0x3e0000);
+ end = (void *)addr + swapsize;
while (addr < end) {
bswap32s(addr);
addr++;
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 04245b5..bcac204 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -169,9 +169,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 == KERN_IMAGE_START || pentry == 0x800) {
+ char *parm_area = rom_ptr(KERN_PARM_AREA, strlen(ipl->cmdline) + 1);
ipl->start_addr = 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 = pentry;
}
@@ -179,6 +182,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
if (ipl->initrd) {
ram_addr_t initrd_offset;
int initrd_size;
+ uint64_t *romptr;
initrd_offset = INITRD_START;
while (kernel_size + 0x100000 > initrd_offset) {
@@ -195,8 +199,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 = 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 0ee779f..2375cb2 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -272,7 +272,7 @@ static unsigned long sun4m_load_kernel(const char *kernel_filename,
}
if (initrd_size > 0) {
for (i = 0; i < 64 * TARGET_PAGE_SIZE; i += TARGET_PAGE_SIZE) {
- ptr = rom_ptr(KERNEL_LOAD_ADDR + i);
+ ptr = rom_ptr(KERNEL_LOAD_ADDR + i, 24);
if (ldl_p(ptr) == 0x48647253) { // HdrS
stl_p(ptr + 16, INITRD_LOAD_ADDR);
stl_p(ptr + 20, initrd_size);
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 1bede85..8b09090 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -186,7 +186,7 @@ static uint64_t sun4u_load_kernel(const char *kernel_filename,
}
if (*initrd_size > 0) {
for (i = 0; i < 64 * TARGET_PAGE_SIZE; i += TARGET_PAGE_SIZE) {
- ptr = rom_ptr(*kernel_addr + i);
+ ptr = rom_ptr(*kernel_addr + i, 32);
if (ldl_p(ptr + 8) == 0x48647253) { /* HdrS */
stl_p(ptr + 24, *initrd_addr + *kernel_addr);
stl_p(ptr + 28, *initrd_size);
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 5ed3fd8..e98b84b 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);
#define rom_add_file_fixed(_f, _a, _i) \
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index ab047b9..95f92b2 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -232,7 +232,7 @@ static void arm_cpu_reset(CPUState *s)
/* Load the initial SP and PC from offset 0 and 4 in the vector table */
vecbase = env->v7m.vecbase[env->v7m.secure];
- rom = rom_ptr(vecbase);
+ rom = rom_ptr(vecbase, 8);
if (rom) {
/* Address zero is covered by ROM which hasn't yet been
* copied into physical memory.
--
1.8.3.1
On Fri, 15 Jun 2018 08:58:17 +0200 Thomas Huth <thuth@redhat.com> wrote: > 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.txt > Segmentation fault (core dumped) It seems nobody managed to trigger the other places yet? > > 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. Makes sense. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > hw/core/loader.c | 10 +++++----- > hw/mips/mips_malta.c | 6 ++++-- > hw/s390x/ipl.c | 13 ++++++++++--- > hw/sparc/sun4m.c | 2 +- > hw/sparc64/sun4u.c | 2 +- > include/hw/loader.h | 2 +- > target/arm/cpu.c | 2 +- > 7 files changed, 23 insertions(+), 14 deletions(-) > Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On 15.06.2018 09:57, Cornelia Huck wrote:
> On Fri, 15 Jun 2018 08:58:17 +0200
> Thomas Huth <thuth@redhat.com> wrote:
>
>> 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.txt
>> Segmentation fault (core dumped)
>
> It seems nobody managed to trigger the other places yet?
mips_malta.c already seems to check bios_size, so it's not affected as
far as I can tell.
But you can crash sparc64 like this:
$ echo -n HdrS > /tmp/test.txt
$ sparc64-softmmu/qemu-system-sparc64 -kernel /tmp/test.txt \
-initrd /tmp/test.txt
Segmentation fault (core dumped)
I was not able to crash sparc32 in the same way, but you can see the bad
access with valgrind there:
$ valgrind sparc-softmmu/qemu-system-sparc -kernel /tmp/test.txt -initrd
/tmp/test.txt
==14301== Memcheck, a memory error detector
==14301== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==14301== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==14301== Command: sparc-softmmu/qemu-system-sparc -kernel /tmp/test.txt
-initrd /tmp/test.txt
==14301==
==14301== Invalid write of size 4
==14301== at 0x2C8477: memcpy (string3.h:51)
==14301== by 0x2C8477: stl_he_p (bswap.h:342)
==14301== by 0x2C8477: stl_be_p (bswap.h:449)
==14301== by 0x2C8477: sun4m_load_kernel (sun4m.c:277)
==14301== by 0x2C8477: sun4m_hw_init (sun4m.c:992)
==14301== by 0x3272F9: machine_run_board_init (machine.c:830)
==14301== by 0x23F809: main (vl.c:4477)
==14301== Address 0x2702fac0 is 12 bytes after a block of size 4 alloc'd
==14301== at 0x4C2B955: calloc (vg_replace_malloc.c:711)
==14301== by 0x77B5F55: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5400.2)
==14301== by 0x3282A8: rom_add_file (loader.c:950)
==14301== by 0x3285EB: load_image_targphys_as (loader.c:153)
==14301== by 0x2C8BDD: sun4m_load_kernel (sun4m.c:253)
==14301== by 0x2C8BDD: sun4m_hw_init (sun4m.c:992)
==14301== by 0x3272F9: machine_run_board_init (machine.c:830)
==14301== by 0x23F809: main (vl.c:4477)
==14301==
==14301== Invalid write of size 4
==14301== at 0x2C847E: memcpy (string3.h:51)
==14301== by 0x2C847E: stl_he_p (bswap.h:342)
==14301== by 0x2C847E: stl_be_p (bswap.h:449)
==14301== by 0x2C847E: sun4m_load_kernel (sun4m.c:278)
==14301== by 0x2C847E: sun4m_hw_init (sun4m.c:992)
==14301== by 0x3272F9: machine_run_board_init (machine.c:830)
==14301== by 0x23F809: main (vl.c:4477)
==14301== Address 0x2702fac4 is 16 bytes after a block of size 4 alloc'd
==14301== at 0x4C2B955: calloc (vg_replace_malloc.c:711)
==14301== by 0x77B5F55: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.5400.2)
==14301== by 0x3282A8: rom_add_file (loader.c:950)
==14301== by 0x3285EB: load_image_targphys_as (loader.c:153)
==14301== by 0x2C8BDD: sun4m_load_kernel (sun4m.c:253)
==14301== by 0x2C8BDD: sun4m_hw_init (sun4m.c:992)
==14301== by 0x3272F9: machine_run_board_init (machine.c:830)
==14301== by 0x23F809: main (vl.c:4477)
==14301==
Thomas
On 15.06.2018 08:58, Thomas Huth wrote:
> 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.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.
[...]
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 0ee779f..2375cb2 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -272,7 +272,7 @@ static unsigned long sun4m_load_kernel(const char *kernel_filename,
> }
> if (initrd_size > 0) {
> for (i = 0; i < 64 * TARGET_PAGE_SIZE; i += TARGET_PAGE_SIZE) {
> - ptr = rom_ptr(KERNEL_LOAD_ADDR + i);
> + ptr = rom_ptr(KERNEL_LOAD_ADDR + i, 24);
> if (ldl_p(ptr) == 0x48647253) { // HdrS
Darn, that should check for ptr != NULL ...
> stl_p(ptr + 16, INITRD_LOAD_ADDR);
> stl_p(ptr + 20, initrd_size);
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 1bede85..8b09090 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -186,7 +186,7 @@ static uint64_t sun4u_load_kernel(const char *kernel_filename,
> }
> if (*initrd_size > 0) {
> for (i = 0; i < 64 * TARGET_PAGE_SIZE; i += TARGET_PAGE_SIZE) {
> - ptr = rom_ptr(*kernel_addr + i);
> + ptr = rom_ptr(*kernel_addr + i, 32);
> if (ldl_p(ptr + 8) == 0x48647253) { /* HdrS */
... dito ...
I'll send a v2.
Thomas
© 2016 - 2024 Red Hat, Inc.