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 <zheyum...@gmail.com>
---
Changes in v3:
- Also change the read operation
- Refine the error log
Changes in v2:
- Also check the memory_region_size bound
---
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 26dc8170d8..af9765a354 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;
--
2.34.1
