linux-kernel - RE: [PATCH] vfio/pci: Disable qword access to the PCI ROM bar

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <BN9PR11MB527626BAEB61201D6D464EFE8CA8A@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Thu, 18 Dec 2025 06:35:29 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Alex Williamson <alex@...zbot.org>
CC: Ankit Agrawal <ankita@...dia.com>, Jason Gunthorpe <jgg@...pe.ca>, "Yishai
 Hadas" <yishaih@...dia.com>, Shameer Kolothum <skolothumtho@...dia.com>,
	Ramesh Thomas <ramesh.thomas@...el.com>, Yunxiang Li <Yunxiang.Li@....com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Chen, Farrah" <farrah.chen@...el.com>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH] vfio/pci: Disable qword access to the PCI ROM bar

> From: Alex Williamson <alex@...zbot.org>
> Sent: Thursday, December 18, 2025 6:08 AM
> 
> On Fri, 12 Dec 2025 02:09:41 +0000
> Kevin Tian <kevin.tian@...el.com> wrote:
> 
> > Commit 2b938e3db335 ("vfio/pci: Enable iowrite64 and ioread64 for vfio
> > pci") enables qword access to the PCI bar resources. However certain
> > devices (e.g. Intel X710) are observed with problem upon qword accesses
> > to the rom bar, e.g. triggering PCI aer errors.
> >
> > Instead of trying to identify all broken devices, universally disable
> > qword access to the rom bar i.e. going back to the old way which worked
> > reliably for years.
> 
> Thanks for finding the root cause of this, Kevin.  I think it would add
> useful context in the commit log here to describe that the ROM is
> somewhat unique in this respect because it's cached by QEMU which
> simply does a pread() of the remaining size until it gets the full
> contents.  The other BARs would only perform operations at the same
> access width as their guest drivers.

will do

> >  ssize_t vfio_pci_core_do_io_rw(struct vfio_pci_core_device *vdev, bool
> test_mem,
> >  			       void __iomem *io, char __user *buf,
> >  			       loff_t off, size_t count, size_t x_start,
> > -			       size_t x_end, bool iswrite)
> > +			       size_t x_end, bool iswrite, bool allow_qword)
> 
> I've been trying to think about how we avoid yet another bool arg here.
> What do you think about creating an enum:
> 
> enum vfio_pci_io_width {
> 	VFIO_PCI_IO_WIDTH_1 = 1,
> 	VFIO_PCI_IO_WIDTH_2 = 2,
> 	VFIO_PCI_IO_WIDTH_4 = 4,
> 	VFIO_PCI_IO_WIDTH_8 = 8,
> };
> 
> The arg here would then be enum vfio_pci_io_width max_width, and for
> each test we'd add a condition && max_width >= 8 (4, 2), where we can
> assume byte access as the minimum regardless of the arg.  It's a little
> more than we need, but it follows a simple pattern and I think makes
> the call sites a bit more intuitive.

make sense

> > @@ -352,7 +363,7 @@ ssize_t vfio_pci_vga_rw(struct vfio_pci_core_device
> *vdev, char __user *buf,
> >  	 * to the memory enable bit in the command register.
> >  	 */
> >  	done = vfio_pci_core_do_io_rw(vdev, false, iomem, buf, off, count,
> > -				      0, 0, iswrite);
> > +				      0, 0, iswrite, true);
> 
> I have no basis other than paranoia and "VGA is old and a 64-bit access
> to it seem wrong", but I'm tempted to restrict this to dword access as
> well.  I don't want to take your fix off track from it's specific goal
> though.  Thanks,
> 

I'll add a new patch for it.