From: Tristan Madani <[email protected]>
The G364 framebuffer allows a guest to set top_of_screen to any
32-bit value without validation against the VRAM size. The display
update function g364fb_draw_graphic8() computes its read pointer as
memory_region_get_ram_ptr() + top_of_screen, reading past the
allocated VRAM buffer when top_of_screen exceeds vram_size.
The developers flagged this exact issue with an inline XXX comment
("XXX: out of range in vram?") but never added the check.
Validate the full display window (top_of_screen + width * height)
against vram_size in g364fb_draw_graphic8(), using 64-bit arithmetic
since all three values are guest-controlled 32-bit registers. This
covers changes to any of the three parameters regardless of the
order the guest programs them.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4648
Cc: [email protected]
Signed-off-by: Tristan Madani <[email protected]>
---
Changes in v2:
- Validate full display window (top_of_screen + width * height),
not just the base address, per Marc-André Lureau
- Use 64-bit arithmetic to prevent overflow of guest-controlled
values
- Move validation to draw function where all three parameters
are known, dropping the insufficient write-handler clamp
hw/display/g364fb.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c
index af54f1f..cc5b31d 100644
--- a/hw/display/g364fb.c
+++ b/hw/display/g364fb.c
@@ -126,7 +126,11 @@ static void g364fb_draw_graphic8(G364State *s)
}
vram = memory_region_get_ram_ptr(&s->mem_vram) + s->top_of_screen;
- /* XXX: out of range in vram? */
+ if ((uint64_t)s->top_of_screen +
+ (uint64_t)s->width * s->height > s->vram_size) {
+ return;
+ }
+
data_display = dd = surface_data(surface);
snap = memory_region_snapshot_and_clear_dirty(&s->mem_vram, 0,
s->vram_size,
DIRTY_MEMORY_VGA);
--
2.47.3
