From c9fe56393bdeb31c38b025a39b6ffa53b5f8d870 Mon Sep 17 00:00:00 2001 From: Zheyu Ma Date: Wed, 17 Jul 2024 16:20:29 +0200 Subject: [PATCH] hw/display/sm501: Fix potential overflow in memory size index This patch addresses potential issues in the sm501 memory size configuration. Specifically, it adds validation checks to ensure that the local_mem_size_index value written to the SM501_DRAM_CONTROL register is within valid bounds. It ensures that the selected memory size does not exceed the available VRAM size. Additionally, the read operation for the SM501_DRAM_CONTROL register is modified to return the raw dram_control value without combining it with the local_mem_size_index. ASAN log: ==3067247==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55c6586e4d3c at pc 0x55c655d4e0ac bp 0x7ffc9d5c6a10 sp 0x7ffc9d5c6a08 READ of size 4 at 0x55c6586e4d3c thread T0 #0 0x55c655d4e0ab in sm501_2d_operation qemu/hw/display/sm501.c:729:21 #1 0x55c655d4b8a1 in sm501_2d_engine_write qemu/hw/display/sm501.c:1551:13 Reproducer: cat << EOF | qemu-system-x86_64 \ -display none -machine accel=qtest, -m 512M -machine q35 -nodefaults \ -device sm501 -qtest stdio outl 0xcf8 0x80000814 outl 0xcfc 0xe4000000 outl 0xcf8 0x80000804 outw 0xcfc 0x02 writel 0xe4000010 0xe000 writel 0xe4100010 0x10000 writel 0xe4100008 0x10001 writel 0xe410000c 0x80000000 EOF Signed-off-by: Zheyu Ma Message-Id: <20240717142029.66372-1-zheyuma97@gmail.com> --- hw/display/sm501.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/hw/display/sm501.c b/hw/display/sm501.c index 26dc8170d89b..af9765a354a1 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -961,7 +961,7 @@ static uint64_t sm501_system_config_read(void *opaque, hwaddr addr, ret = 0x050100A0; break; case SM501_DRAM_CONTROL: - ret = (s->dram_control & 0x07F107C0) | s->local_mem_size_index << 13; + ret = s->dram_control; break; case SM501_ARBTRTN_CONTROL: ret = s->arbitration_control; @@ -1020,11 +1020,24 @@ static void sm501_system_config_write(void *opaque, hwaddr addr, s->gpio_63_32_control = value & 0xFF80FFFF; break; case SM501_DRAM_CONTROL: - s->local_mem_size_index = (value >> 13) & 0x7; - /* TODO : check validity of size change */ + { + int local_mem_size_index = (value >> 13) & 0x7; + if (local_mem_size_index >= ARRAY_SIZE(sm501_mem_local_size)) { + qemu_log_mask(LOG_GUEST_ERROR, + "sm501: Invalid local_mem_size_index value: %d\n", + local_mem_size_index); + } else if (sm501_mem_local_size[local_mem_size_index] > + memory_region_size(&s->local_mem_region)) { + qemu_log_mask(LOG_GUEST_ERROR, + "sm501: Memory size %d cannot be more than vram_size\n", + sm501_mem_local_size[local_mem_size_index]); + } else { + s->local_mem_size_index = local_mem_size_index; + } s->dram_control &= 0x80000000; s->dram_control |= value & 0x7FFFFFC3; break; + } case SM501_ARBTRTN_CONTROL: s->arbitration_control = value & 0x37777777; break;