Re: [PATCH] drm/cirrus-qemu: Validate BAR0 size during probe
Slawomir Stepien <[email protected]>
| Newsgroups | org.freedesktop.lists.dri-devel,dev.linux.lists.sashiko-reviews,dev.linux.lists.syzbot |
|---|---|
| Message-ID | <ao18Ag6ucRMqGRVI@nr200> |
On sie 25, 2026 12:21, Thomas Zimmermann wrote: > Hi > > Am 25.08.26 um 11:46 schrieb Slawomir Stepien: > > 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 [email protected]: > > > > 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 <[email protected]> > > > > > > > > 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. > > Please check that it is >= VRAM_SIZE, so we're flexible. Sure! -- Slawomir Stepien