linux-kernel - Re: [PATCH v3 2/3] vfio/pci: Tolerate oversized BARs by disallowing mmap

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]

Date: Thu, 20 Jun 2024 14:29:38 +0200
From: Gerd Bayer <gbayer@...ux.ibm.com>
To: Niklas Schnelle <schnelle@...ux.ibm.com>,
        Christoph Hellwig
	 <hch@...radead.org>
Cc: Alex Williamson <alex.williamson@...hat.com>,
        Gerald Schaefer
 <gerald.schaefer@...ux.ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>, Vasily
 Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Sven Schnelle
 <svens@...ux.ibm.com>,
        Matthew Rosato <mjrosato@...ux.ibm.com>,
        Jason
 Gunthorpe <jgg@...pe.ca>, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v3 2/3] vfio/pci: Tolerate oversized BARs by disallowing
 mmap

On Thu, 2024-06-20 at 14:06 +0200, Niklas Schnelle wrote:
> On Wed, 2024-06-19 at 21:09 -0700, Christoph Hellwig wrote:
> > On Wed, Jun 19, 2024 at 12:56:47PM +0200, Niklas Schnelle wrote:
> > > In short, the ISM BAR 0 is stupidly large but this is
> > > intentional. It not fitting in the VMAP is simply the least crazy
> > > filter I could come up with to keep the ISM device from causing
> > > trouble for use of vfio-pci mmap() for other, normal, PCI
> > > devices.
> > 
> > Then maybe add a PCI quirk to prevent mapping it.  This would also
> > affect the sysfs resource0 file unless I'm missing something.
> > 
> 
> The resource<N> files are globally disabled on s390x due to lack of
> HAVE_PCI_MMAP/ARCH_GENERIC_PCI_MMAP_RESOURCE at the moment. I might
> change that in the future with a analogous argument as for
> VFIO_PCI_MMAP but for its not there. Once we add it we of course need
> the same special ISM treatment there too.
> 
> Looking at the existing PCI quirks I don't see anything that would
> fit so I'm guessing you would want to add a new quirk? As I
> understand it we would then have to export something like a
> is_pci_mmap_broken(pdev) function while currently the only quirk
> function that seems to be exported is pxi_fixup_device(). But then
> what happens if CONFIG_PCI_QUIRKS=n? Also the header comment in
> drivers/pci/quirks.c says that platform specific devices shouldn't go
> in there and ISM is platform specific.

I took a cursory look at that file as well and arrived at a similar
conclusion that drivers/pci/quirks.c (as is) does not sound like a good
match for the required functionality


> Instead of exporting IOREMAP_START/IOREMAP_END maybe there is another
> reasonable maximum resource length? Or maybe we could create a size_t
> ioremap_area_size() helper similar to is_ioremap_addr() but not
> inlined. The latter already uses IOREMAP_START/IOREMAP_END so not
> sure how that works when IOREMAP_END is not exported?

But seeing how the PCI quirks filter against Vendor and Device ID,
I just had the idea if it would make sense to create a similar
infrastructure in the form of VFIO PCI quirks - and simply reject mmap
on ISM devices regardless of the size of iomap'able address spaces?
Sound rather coarse-grained, though...

> Thanks,
> Niklas

Just a thought,
Gerd