Re: [PATCH] drm/cirrus-qemu: Validate BAR0 size during probe

[Slawomir Stepien <sst@poczta.fm>]

On sie 25, 2026 10:00, Thomas Zimmermann wrote:
> (cc: Slawomir)
> 
> Hi,

Hello,

> thanks for the fix. Please see below for changes that should go into the
> patch.

thanks I will look into them and make a manual v2!

> Best regards
> Thomas
> 
> Am 25.08.26 um 09:44 schrieb sashiko-bot@kernel.org:
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [High] The exact equality check for BAR0 size (`!= CIRRUS_VRAM_SIZE`) incorrectly rejects valid QEMU configurations with larger VRAM.
> > - [High] The VRAM size validation check is placed after `aperture_remove_conflicting_pci_devices()`, meaning the firmware framebuffer is permanently destroyed even if the driver ultimately fails to bind.
> > --
> > 
> > --- Patch [1]: [PATCH] drm/cirrus-qemu: Validate BAR0 size during probe ---
> > commit 75467ead62a494b6afbc057c666ab59f7e4b8915
> > Author: Slawomir Stepien <sst@poczta.fm>
> > 
> > drm/cirrus-qemu: Validate BAR0 size during probe
> > 
> > This commit adds a check during PCI probe to verify that the BAR0 resource
> > length equals CIRRUS_VRAM_SIZE. This is intended to prevent supervisor write
> > page faults caused by writing past the end of the mapped I/O memory when
> > smaller BARs are presented.
> > 
> > Link: https://syzkaller.appspot.com/ai_job?id=ba262a3a-bccf-4ad8-a1b0-583c55d34fd6
> > 
> > > diff --git a/drivers/gpu/drm/tiny/cirrus-qemu.c b/drivers/gpu/drm/tiny/cirrus-qemu.c
> > > index 075221b431d37..70c995025f508 100644
> > > --- a/drivers/gpu/drm/tiny/cirrus-qemu.c
> > > +++ b/drivers/gpu/drm/tiny/cirrus-qemu.c
> > > @@ -602,6 +602,9 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
> > >   	dev = &cirrus->dev;
> > > +	if (pci_resource_len(pdev, 0) != CIRRUS_VRAM_SIZE)
> > > +		return -ENODEV;
> > > +
> > [Severity: High]
> > Does this strict equality check unintentionally break valid QEMU
> > configurations that have more than 4 MB of VRAM?
> > 
> > When a VM is booted with a configuration like -device cirrus-vga,vgamem_mb=8,
> > the BAR0 size will be 8 MB. The check in cirrus_pci_probe() evaluates
> > to true (since 8 MB is not equal to 4 MB) and rejects the device by
> > returning -ENODEV.
> > 
> > Would it be safer to check if pci_resource_len(pdev, 0) < CIRRUS_VRAM_SIZE
> > instead of requiring an exact match?

It seems that 4MB is the default one and 8 and 16MB are just for compatibility:

(qemu hw/display/cirrus_vga.c):

2951     /*
2952      * Follow real hardware, cirrus card emulated has 4 MB video memory.
2953      * Also accept 8 MB/16 MB for backward compatibility.
2954      */
2955     if (s->vga.vram_size_mb != 4 && s->vga.vram_size_mb != 8 &&
2956         s->vga.vram_size_mb != 16) {
2957         error_setg(errp, "Invalid cirrus_vga ram size '%u'",
2958                    s->vga.vram_size_mb);
2959         return;
2960     }

I will add the 8 and 16MB then in v2.

> > [Severity: High]
> > Is it safe to place this validation check after the call to
> > aperture_remove_conflicting_pci_devices() earlier in cirrus_pci_probe()?
> > 
> > If this new size check fails (either due to a malformed device or a larger
> > valid VRAM size), cirrus_pci_probe() aborts with -ENODEV. However, because
> > aperture_remove_conflicting_pci_devices() was already executed at the start
> > of the probe function, the firmware framebuffers (like efifb or simplefb)
> > have been permanently destroyed.
> > 
> > Could this leave the system with a dead display output? Would it make sense
> > to move this PCI configuration check to the very beginning of the probe
> > function, before any destructive actions occur?

Yeah I guess this new check can be before aperture_remove_conflicting_pci_devices().

> > >   	cirrus->vram = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0),
> > >   				    pci_resource_len(pdev, 0));
> > >   	if (cirrus->vram == NULL)

-- 
Slawomir Stepien