[PATCH 0/2] hw/display/cirrus_vga: Fix packed-24 color-expansion ops

Peter Maydell posted 2 patches 20 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260410183249.4046456-1-peter.maydell@linaro.org
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
hw/display/cirrus_vga_rop2.h | 52 ++++++++++++++++++++++++++++++++----
1 file changed, 47 insertions(+), 5 deletions(-)
[PATCH 0/2] hw/display/cirrus_vga: Fix packed-24 color-expansion ops
Posted by Peter Maydell 20 hours ago
The Cirrus Logic VGA card raster ops allow the left side of the input
source to be clipped (or skipped) based on a "destination write mask"
field in the GR2F bitblt register. For 8, 16 and 32bpp, this field has
a simple 3-bit format indicating the number of pixels to skip on the
left edge of each scanline. For 24bpp, the field is 5-bit, and is a
byte count of how many destination bytes to skip.

The card also supports a "color expansion" mode for raster ops where
the input is a monochrome bitmap or bitmap pattern, where each bit is
expanded into a full-color pixel which is either the background or
foreground color (or not drawn at all, for transparency).

We have a bug in our implementation of the interaction of these two
features: for color-expansion to 8/16/32bpp the 3-bit destination
write mask field doesn't allow skipping a full byte of the input
monochrome bitmap, but in 24bpp the 5-bit field means we might need to
skip a byte and a bit of the source. We weren't accounting for this
need to skip a full byte. The result for the "pattern fill" raster op
type (where the input is a repeating 8x8 monochrome tile) is that we
attempt to shift by a negative value, which is caught by the
undefined-behaviour sanitizer. For the other color-expansion ops, we
merely write the wrong data to the display.

This patchset fixes both bugs. The UB one was reported as
https://gitlab.com/qemu-project/qemu/-/work_items/3377

The technical reference manual says that 24-bpp color expansion
"*must* use transparency" and doesn't say what happens if you try to
do it anyway, which is presumably why the other raster op functions in
cirrus_vga_rop2.h don't try to handle the 24bpp write-mask format. I
have left this as it is, since we don't have any reports of UB there.

Big disclaimer on this: I have no guest images that try to use the
Cirrus bitblt handling at all, so I have only developed these against
the reference manual and confirmed that it fixes the UB repro case
from the bug. If anybody does have test images for these that would be
good to test...

This bug has been in QEMU for decades, so I think we're OK letting
it slide to QEMU 11.1.

thanks
-- PMM

Peter Maydell (2):
  hw/display/cirrus_vga: Fix packed-24 color-expansion transparent
    pattern fills
  hw/display/cirrus_vga: Fix packed-24 color-expansion transparent
    copies

 hw/display/cirrus_vga_rop2.h | 52 ++++++++++++++++++++++++++++++++----
 1 file changed, 47 insertions(+), 5 deletions(-)

-- 
2.43.0