When a ROM blob runs past the end of the destination, rom_copy()
clamps the length to "dest - d" instead of "dest + size - d". The
result is negative, so memcpy() gets a huge size_t and QEMU crashes.
The same mistake is in the memset() path.
This is easy to hit with a crafted -kernel image. For example, a
multiboot ELF whose last segment ends right where the BIOS starts
(0xfffc0000 with the default SeaBIOS) crashes qemu-system-i386 at
startup.
While at it, treat the range as half-open at both ends: a blob that
starts at addr + size now ends the loop, so rom_copy() no longer
reports a full copy when nothing was copied, and a blob that ends
exactly at addr no longer stops the loop before the blobs inside the
range.
Commit e423455c4f23 fixed a similar problem at the start of the
range.
Fixes: 235f86ef0147 ("Introduce rom_copy")
Fixes: d60fa42e8bae ("Save memory allocation in the elf loader")
Signed-off-by: Dominik M. Weber <[email protected]>
---
hw/core/loader.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 5cbfba0a86..d293603cad 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1529,10 +1529,10 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size)
if (rom->mr) {
continue;
}
- if (rom->addr + rom->romsize < addr) {
+ if (rom->addr + rom->romsize <= addr) {
continue;
}
- if (rom->addr > end || rom->addr < addr) {
+ if (rom->addr >= end || rom->addr < addr) {
break;
}
@@ -1541,7 +1541,7 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size)
l = rom->datasize;
if ((d + l) > (dest + size)) {
- l = dest - d;
+ l = dest + size - d;
}
if (l > 0) {
@@ -1560,7 +1560,7 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size)
/* Rom size doesn't fit in the destination area. Adjust to
avoid
* overflow.
*/
- l = dest - d;
+ l = dest + size - d;
}
if (l > 0) {
--
2.43.0
