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] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250826-pci_fix_sriov_disable-v1-1-2d0bc938f2a3@linux.ibm.com>
Date: Tue, 26 Aug 2025 10:52:08 +0200
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: Bjorn Helgaas <helgaas@...nel.org>, Lukas Wunner <lukas@...ner.de>
Cc: Keith Busch <kbusch@...nel.org>, Gerd Bayer <gbayer@...ux.ibm.com>,
        Matthew Rosato <mjrosato@...ux.ibm.com>,
        Benjamin Block <bblock@...ux.ibm.com>,
        Halil Pasic <pasic@...ux.ibm.com>, Farhan Ali <alifm@...ux.ibm.com>,
        Julian Ruess <julianr@...ux.ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, Niklas Schnelle <schnelle@...ux.ibm.com>
Subject: [PATCH 1/2] PCI/IOV: Add missing PCI rescan-remove locking when
 enabling/disabling SR-IOV

Before disabling SR-IOV via config space accesses to the parent PF,
sriov_disable() first removes the PCI devices representing the VFs.

Since commit 9d16947b7583 ("PCI: Add global pci_lock_rescan_remove()")
such removal operations are serialized against concurrent remove and
rescan using the pci_rescan_remove_lock. No such locking was ever added
in sriov_disable() however. In particular when commit 18f9e9d150fc
("PCI/IOV: Factor out sriov_add_vfs()") factored out the PCI device
removal into sriov_del_vfs() there was still no locking around the
pci_iov_remove_virtfn() calls.

On s390 the lack of serialization in sriov_disable() may cause double
remove and list corruption with the below (amended) trace being observed:

 PSW:  0704c00180000000 0000000c914e4b38 (klist_put+56)
 GPRS: 000003800313fb48 0000000000000000 0000000100000001 0000000000000001
       00000000f9b520a8 0000000000000000 0000000000002fbd 00000000f4cc9480
       0000000000000001 0000000000000000 0000000000000000 0000000180692828
       00000000818e8000 000003800313fe2c 000003800313fb20 000003800313fad8
 #0 [3800313fb20] device_del at c9158ad5c
 #1 [3800313fb88] pci_remove_bus_device at c915105ba
 #2 [3800313fbd0] pci_iov_remove_virtfn at c9152f198
 #3 [3800313fc28] zpci_iov_remove_virtfn at c90fb67c0
 #4 [3800313fc60] zpci_bus_remove_device at c90fb6104
 #5 [3800313fca0] __zpci_event_availability at c90fb3dca
 #6 [3800313fd08] chsc_process_sei_nt0 at c918fe4a2
 #7 [3800313fd60] crw_collect_info at c91905822
 #8 [3800313fe10] kthread at c90feb390
 #9 [3800313fe68] __ret_from_fork at c90f6aa64
 #10 [3800313fe98] ret_from_fork at c9194f3f2.

This is because in addition to sriov_disable() removing the VFs, the
platform also generates hot-unplug events for the VFs. This being
the reverse operation to the hotplug events generated by sriov_enable()
and handled via pdev->no_vf_scan. And while the event processing takes
pci_rescan_remove_lock and checks whether the struct pci_dev still
exists, the lack of synchronization makes this checking racy.

Other races may also be possible of course though given that this lack
of locking persisted so long obversable races seem very rare. Even on
s390 the list corruption was only observed with certain devices since
the platform events are only triggered by the config accesses that come
after the removal, so as long as the removal finnished synchronously
they would not race. Either way the locking is missing so fix this by
adding it to the sriov_del_vfs() helper.

Just lik PCI rescan-remove locking is also missing in sriov_add_vfs()
including for the error case where pci_stop_ad_remove_bus_device() is
called without the PCI rescan-remove lock being held. Even in the non
error case adding new PCI devices and busses should be serialized via
the PCI rescan-remove lock. Add the necessary locking.

Cc: stable@...r.kernel.org
Fixes: 18f9e9d150fc ("PCI/IOV: Factor out sriov_add_vfs()")
Reviewed-by: Benjamin Block <bblock@...ux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@...ux.ibm.com>
---
 drivers/pci/iov.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index ac4375954c9479b5f4a0e666b5215094fdaeefc2..77dee43b785838d215b58db2d22088e9346e0583 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -629,15 +629,18 @@ static int sriov_add_vfs(struct pci_dev *dev, u16 num_vfs)
 	if (dev->no_vf_scan)
 		return 0;
 
+	pci_lock_rescan_remove();
 	for (i = 0; i < num_vfs; i++) {
 		rc = pci_iov_add_virtfn(dev, i);
 		if (rc)
 			goto failed;
 	}
+	pci_unlock_rescan_remove();
 	return 0;
 failed:
 	while (i--)
 		pci_iov_remove_virtfn(dev, i);
+	pci_unlock_rescan_remove();
 
 	return rc;
 }
@@ -762,8 +765,10 @@ static void sriov_del_vfs(struct pci_dev *dev)
 	struct pci_sriov *iov = dev->sriov;
 	int i;
 
+	pci_lock_rescan_remove();
 	for (i = 0; i < iov->num_VFs; i++)
 		pci_iov_remove_virtfn(dev, i);
+	pci_unlock_rescan_remove();
 }
 
 static void sriov_disable(struct pci_dev *dev)

-- 
2.48.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ