linux-kernel - Re: [PATCH] pci: account for sysfs-disabled reset in pci_{slot,bus}

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: <Z5JczeFgDtX35nV3@6121402-lcelt>
Date: Thu, 23 Jan 2025 09:14:21 -0600
From: Nishanth Aravamudan <naravamudan@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>,
	Alex Williamson <alex.williamson@...hat.com>,
	Raphael Norwitz <raphael.norwitz@...anix.com>,
	Amey Narkhede <ameynarkhede03@...il.com>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, Yishai Hadas <yishaih@...dia.com>,
	Shameer Kolothum <shameerali.kolothum.thodi@...wei.com>,
	Kevin Tian <kevin.tian@...el.com>, kvm@...r.kernel.org
Subject: Re: [PATCH] pci: account for sysfs-disabled reset in
 pci_{slot,bus}_resettable

On Thu, Jan 23, 2025 at 09:33:12AM -0400, Jason Gunthorpe wrote:
> On Wed, Jan 22, 2025 at 12:14:02PM -0600, Nishanth Aravamudan wrote:
> > On Mon, Jan 13, 2025 at 04:42:00PM -0400, Jason Gunthorpe wrote:
> > > On Mon, Jan 06, 2025 at 03:52:31PM -0600, Nishanth Aravamudan wrote:
> > > > vfio_pci_ioctl_get_pci_hot_reset_info checks if either the vdev's slot
> > > > or bus is not resettable by calling pci_probe_reset_{slot,bus}. Those
> > > > functions in turn call pci_{slot,bus}_resettable() to see if the PCI
> > > > device supports reset.
> > > 
> > > This change makes sense to me, but..
> > > 
> > > > However, commit d88f521da3ef ("PCI: Allow userspace to query and set
> > > > device reset mechanism") added support for userspace to disable reset of
> > > > specific PCI devices (by echo'ing "" into reset_method) and
> > > > pci_{slot,bus}_resettable methods do not check pci_reset_supported() to
> > > > see if userspace has disabled reset. Therefore, if an administrator
> > > > disables PCI reset of a specific device, but then uses vfio-pci with
> > > > that device (e.g. with qemu), vfio-pci will happily end up issuing a
> > > > reset to that device.
> > > 
> > > How does vfio-pci endup issuing a reset? It looked like all the paths
> > > are blocked in the pci core with pci_reset_supported()? Is there also
> > > a path that vfio is calling that is missing a pci_reset_supported()
> > > check? If yes that should probably be fixed in another patch.
> > 
> > This is the path I observed:
> 
> You didn't answer the question, I didn't ask about pci_probe_*() I
> asked why doesn't pci_reset_supported() directly block the actual
> reset?

Sorry, I misunderstood your question.

__pci_reset_bus()
	-> pci_bus_reset(..., PCI_RESET_PROBE)
		-> pci_bus_resettable()

__pci_reset_slot()
	-> pci_slot_reset(..., PCI_RESET_PROBE)
		-> pci_slot_resettable()

pci_reset_bus()
	-> pci_probe_reset_slot()
		-> pci_slot_reset(..., PCI_RESET_PROBE)
			-> pci_bus_resettable()
	if true:
		__pci_reset_slot()
	else:
		__pci_reset_bus()

Before my change, both call paths would end up calling
pci_slot_resettable() and not checking the sysfs file-contents.

Please let me know if that addresses your concern, I think my changes
fixes the paths you are talking about as well. If I need to clarify this
in the commit message, I can.

-Nish