Re: [PATCH] vga: split text renderer geometry cache from graphics renderer
Warisjeet Singh (sin99xx) <[email protected]>
| Newsgroups | org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
Hi Marc-André, On Sat, Aug 22, 2026, Marc-André Lureau wrote: > No worries, but drop it from the commit message, or use '---' > (three-dashes, see git-am(1)) section instead. Done — v2 keeps notes under '---' only. > Also add > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4215 Added. > But the patch doesn't fix the test you reported though.. Can you > check? Compile qemu with ASAN. You were right, and I found the exact reason. The split alone is insufficient because the text predicate still can't observe the console surface being replaced by the graphics renderer: - G1 in my reproducer is a *legacy* (non-VBE) graphics mode, and vga_get_bpp() returns 0 there — so s->last_depth stays 0 and the "|| s->last_depth" term that normally forces a resize after a graphics frame never fires. - With v1, last_text_width/last_text_height still hold (80, 25) from the first text frame, so the T2 predicate compares equal, the resize is skipped, and the glyph loop paints 720x400 px into the 80x25 px surface the graphics path left behind. v2 keeps the split (units are now unambiguous) and adds the term that actually catches the swap: the text path compares the pixel size it is about to paint (width*cw x height*cheight) against the console surface's real dimensions, and resizes on mismatch. Caches in either unit can be stale wrt the surface; the surface cannot. Verification (master @ eea8fe61b8 and v11.1.0, same qtest PoC as in the report): - unpatched: SIGSEGV in vga_draw_glyph9() during the T2 render. - v2 patched: T2 forces the console resize, QEMU survives, subsequent screendumps work. - ASAN build with qemu_pixman_shareable_alloc()/free() routed to g_malloc() as you suggested: unpatched, the PoC triggers "heap-buffer-overflow WRITE of size 4" in vga_draw_glyph9() (vga-helpers.h:80), 0 bytes after the 8000-byte surface region — fired through the ordinary console refresh BH, no screendump needed. With v2 applied the same run is clean (no ASAN report, all redraws succeed). v2 sent as a new thread: [PATCH v2] hw/display/vga: fix text-mode OOB write after a graphics surface switch. Regards, Warisjeet