linux-kernel - Re: [PATCH 1/2] PCI/IOV: Add missing PCI rescan-remove locking when enabling/disabling SR-IOV

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: <DD2MGKOWJ4G1.2QC87YOMH7T63@linux.ibm.com>
Date: Fri, 26 Sep 2025 11:24:52 +0200
From: "Julian Ruess" <julianr@...ux.ibm.com>
To: "Niklas Schnelle" <schnelle@...ux.ibm.com>,
        "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>
Subject: Re: [PATCH 1/2] PCI/IOV: Add missing PCI rescan-remove locking when
 enabling/disabling SR-IOV

On Tue Aug 26, 2025 at 10:52 AM CEST, Niklas Schnelle wrote:
> 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)

Feel free to add my
Reviewed-by: Julian Ruess <julianr@...ux.ibm.com>

Thanks,
Julian