@samitouri / QOSamiQemu / commits / 2ac2cf0483

hw/display/sm501: Don't allow guest to set ram size larger than it is

The SM501 DRAM_CONTROL register has a 7 bit Size field which allows the guest to change the local memory size. We use the local memory size in bounds checks calculations for 2D operations. Currently we have no check on the validity of the value the guest programs to this field, which means that the guest can: - set it to a reserved value (6 or 7) which will cause get_local_mem_size() to read outside sm501_mem_local_size[] - set it to a value corresponding to more RAM than the card was created with, so that the 2D bounds check will let 2D operations access off the end of the memory region Fix this by decoupling the value the guest reads and writes to this field from the internal size we consider the local memory to have. We validate changes and ignore them except for readback if they would be reserved values or values for more memory than the card has. Cc: qemu-stable@nongnu.org Reported-by: Heechan Kang Tested-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20260707150933.1410507-4-peter.maydell@linaro.org Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3811 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Peter Maydell committed Jul 13, 2026 at 12:34 UTC 2ac2cf0483d090f2f2d795ed06a94cc865b972fe

1 file changed +21 -3

hw/display/sm501.c

+21 -3

@@ -571,6 +571,25 @@ static uint32_t get_local_mem_size_index(uint32_t size)

571 return index;

572 }

573

574 +static void set_new_local_mem_size_index(SM501State *s, uint32_t idx)

575 +{

576 + /*

577 + * Update local_mem_size_index on guest write. We don't allow this

578 + * to be set to larger than the actual RAM size. (The guest will

579 + * still read back the SYSTEM_CONTROL.Size bits that it wrote.)

580 + */

581 + if (idx < ARRAY_SIZE(sm501_mem_local_size) &&

582 + sm501_mem_local_size[idx] <= memory_region_size(&s->local_mem_region)) {

583 + s->local_mem_size_index = idx;

584 + return;

585 + }

586 + qemu_log_mask(LOG_GUEST_ERROR,

587 + "sm501: Guest set DRAM_CONTROL.Size to 0x%x but "

588 + "local memory is not that large\n",

589 + idx);

590 + /* Don't change the effective size, leave it as whatever it was */

591 +}

592 +

593 static ram_addr_t get_fb_addr(SM501State *s, int crt)

594 {

595 return (crt ? s->dc_crt_fb_addr : s->dc_panel_fb_addr) & 0x3FFFFF0;

@@ -990,7 +1009,7 @@ static uint64_t sm501_system_config_read(void *opaque, hwaddr addr,

1009 ret = 0x050100A0;

1010 break;

1011 case SM501_DRAM_CONTROL:

993 - ret = (s->dram_control & 0x07F107C0) | s->local_mem_size_index << 13;

1012 + ret = (s->dram_control & 0x07F1E7C0);

1013 break;

1014 case SM501_ARBTRTN_CONTROL:

1015 ret = s->arbitration_control;

@@ -1049,8 +1068,7 @@ static void sm501_system_config_write(void *opaque, hwaddr addr,

1068 s->gpio_63_32_control = value & 0xFF80FFFF;

1069 break;

1070 case SM501_DRAM_CONTROL:

1052 - s->local_mem_size_index = (value >> 13) & 0x7;

1053 - /* TODO : check validity of size change */

1071 + set_new_local_mem_size_index(s, (value >> 13) & 0x7);

1072 s->dram_control &= 0x80000000;

1073 s->dram_control |= value & 0x7FFFFFC3;

1074 break;