From: Tristan Madani <[email protected]> The SM501 display controller allows a guest to set the CRT/Panel framebuffer base address and hardware cursor address to values that can exceed the actual VRAM allocation via the 0x3FFFFF0 mask.
sm501_update_display() reads width * height * src_bpp bytes from the framebuffer base without bounds checking. The hardware cursor path reads 1024 bytes (64x64 at 2bpp) from its base address without bounds checking. The 2D engine path already validates VRAM bounds via sm501_rect_outside_vram(), but the display rendering path has no equivalent check. Add a full extent check in sm501_update_display() that validates framebuffer_addr + width * height * src_bpp against the local memory size using 64-bit arithmetic, and validate the cursor address plus its fixed 1024-byte pattern size in get_hwc_address(). Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4649 Cc: [email protected] Signed-off-by: Tristan Madani <[email protected]> --- Changes in v2: - Validate full framebuffer extent (addr + width * height * bpp), not just the base address, per Marc-André Lureau - Validate cursor address plus its 1024-byte pattern size, not just the base offset - Use 64-bit arithmetic for framebuffer extent check - Move framebuffer validation to sm501_update_display() where width, height, and bpp are all known - Return NULL from get_hwc_address() on bounds failure instead of silently redirecting to address 0 hw/display/sm501.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/hw/display/sm501.c b/hw/display/sm501.c index cacb6c8..c1b13a0 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -630,7 +630,13 @@ static inline int is_hwc_enabled(SM501State *state, int crt) static inline uint8_t *get_hwc_address(SM501State *state, int crt) { uint32_t addr = crt ? state->dc_crt_hwc_addr : state->dc_panel_hwc_addr; - return state->local_mem + (addr & 0x03FFFFF0); + uint32_t offset = addr & 0x03FFFFF0; + + if (offset + SM501_HWC_WIDTH * SM501_HWC_HEIGHT / 4 + > get_local_mem_size(state)) { + return NULL; + } + return state->local_mem + offset; } /** @@ -1815,6 +1821,9 @@ static bool sm501_update_display(void *opaque) /* choose cursor draw line function */ draw_hwc_line = draw_hwc_line_32; hwc_src = get_hwc_address(s, crt); + if (!hwc_src) { + draw_hwc_line = NULL; + } c_x = get_hwc_x(s, crt); c_y = get_hwc_y(s, crt); get_hwc_palette(s, crt, hwc_palette); @@ -1837,6 +1846,9 @@ static bool sm501_update_display(void *opaque) /* draw each line according to conditions */ offset = get_fb_addr(s, crt); + if (offset + (uint64_t)width * height * src_bpp > get_local_mem_size(s)) { + return true; + } snap = memory_region_snapshot_and_clear_dirty(&s->local_mem_region, offset, width * height * src_bpp, DIRTY_MEMORY_VGA); for (y = 0; y < height; y++, offset += width * src_bpp) { -- 2.47.3
