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

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.

Best regards
Thomas

>
>>> [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)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)