dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Slawomir Stepien <sst@poczta.fm>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: sashiko-reviews@lists.linux.dev, syzbot <syzbot@kernel.org>,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/cirrus-qemu: Validate BAR0 size during probe
Date: Tue, 25 Aug 2026 13:26:58 +0200	[thread overview]
Message-ID: <ao18Ag6ucRMqGRVI@nr200> (raw)
In-Reply-To: <7a61b8fc-68f4-4d8d-b5ff-0ffd9a9507fc@suse.de>

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 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.
> 
> Please check that  it is >= VRAM_SIZE, so we're flexible.

Sure!

-- 
Slawomir Stepien

      reply	other threads:[~2026-08-25 14:36 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-25  7:36 [PATCH] drm/cirrus-qemu: Validate BAR0 size during probe syzbot
2026-08-25  7:44 ` sashiko-bot
2026-08-25  8:00   ` Thomas Zimmermann
2026-08-25  9:46     ` Slawomir Stepien
2026-08-25 10:21       ` Thomas Zimmermann
2026-08-25 11:26         ` Slawomir Stepien [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ao18Ag6ucRMqGRVI@nr200 \
    --to=sst@poczta.fm \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=syzbot@kernel.org \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox