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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver maps PCI BAR1 (MMIO) without verifying that its length is sufficient to cover the register offsets accessed by the driver (up to 0x15 for CRT_DATA). If a malicious or malformed PCI device provides a very small BAR1 (e.g., 16 bytes), accesses to higher offsets could cross page boundaries into unmapped memory, potentially causing a kernel panic.
--

--- Patch [1]: [PATCH v2] drm/cirrus-qemu: Validate BAR0 size during probe ---
commit b04af41f1fca891d59ccb1049a9b053cf626fb58
Author: Slawomir Stepien <[email protected]>
drm/cirrus-qemu: Validate BAR0 size during probe

This commit adds a check during probe to validate that the PCI BAR0 size is at
least CIRRUS_VRAM_SIZE. This prevents out-of-bounds memory accesses when
drm_fb_memcpy() copies framebuffers to VRAM mapped from a smaller BAR0.

Link: https://syzkaller.appspot.com/bug?extid=2442951a6abb004df963
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..3bf23fcf65749 100644
> --- a/drivers/gpu/drm/tiny/cirrus-qemu.c
> +++ b/drivers/gpu/drm/tiny/cirrus-qemu.c
> @@ -582,6 +582,9 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
>  	struct cirrus_device *cirrus;
>  	int ret;
>  
> +	if (pci_resource_len(pdev, 0) < CIRRUS_VRAM_SIZE)
> +		return -ENODEV;
> +

[Severity: High]
This is a pre-existing issue, but since we are validating the length of BAR0
(VRAM) here to prevent out-of-bounds accesses, should we also validate the
length of BAR1 (MMIO)?

Later in cirrus_pci_probe(), BAR1 is mapped using its raw length:

    cirrus->mmio = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 1),
                                pci_resource_len(pdev, 1));

The driver subsequently accesses MMIO offsets up to 0x15:

drivers/gpu/drm/tiny/cirrus-qemu.c:wreg_crt() {
    ...
    iowrite8(reg, cirrus->mmio + CRT_INDEX);
    iowrite8(val, cirrus->mmio + CRT_DATA);
}

If a malformed PCI device presents a very small BAR1 (e.g., 16 bytes) and is
allocated such that its physical address is near the end of a page (e.g.,
ending at 0xFFF), devm_ioremap() will map only that single page.

Could accessing offset 0x15 cross the page boundary into unmapped virtual
memory and trigger a supervisor page fault?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1