Re: [PATCH v5 02/18] PCI/CXL: Probe the underlying bus reset in cxl_reset_bus_function()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless traversal of the PCI bus devices list without holding `pci_bus_sem`.
--

commit fc0e8d0d4aa1ff4a78d2d50fab18bdedb164b4ae
Author: Nicolin Chen <nicolinc@nvidia.com>
Subject: PCI/CXL: Probe the underlying bus reset in cxl_reset_bus_function()

This commit updates cxl_reset_bus_function() to properly probe for an
underlying bus reset rather than assuming it is supported. By explicitly
calling pci_dev_reset_slot_function() and pci_parent_bus_reset() on the
probe path, it ensures the sysfs node correctly reflects supported reset
methods. It also passes PCI_RESET_DO_RESET to the actual reset execution
instead of relying on the probe parameter.

> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
[ ... ]
> @@ -4947,8 +4947,16 @@ static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
>  	if (!dvsec)
>  		return -ENOTTY;
>  
> -	if (probe)
> -		return 0;
> +	/*
> +	 * Do not probe via pci_reset_bus_function(), which would reject a
> +	 * masked SBR that the do-reset path below unmasks before resetting.
> +	 */
> +	if (probe) {
> +		rc = pci_dev_reset_slot_function(dev, PCI_RESET_PROBE);
> +		if (rc != -ENOTTY)
> +			return rc;
> +		return pci_parent_bus_reset(dev, PCI_RESET_PROBE);

[Severity: High]
This isn't a bug introduced by this patch, but does calling
pci_parent_bus_reset() here expose us to a lockless list traversal race?

Looking at pci_parent_bus_reset(), it iterates over dev->bus->devices without
holding pci_bus_sem:

drivers/pci/pci.c:pci_parent_bus_reset() {
	...
	list_for_each_entry(pdev, &dev->bus->devices, bus_list)
		if (pdev != dev)
			return -ENOTTY;
	...
}

If a concurrent PCI hotplug operation modifies the bus->devices list under
down_write(&pci_bus_sem), could this lockless iteration dereference poisoned
list pointers? Writing to the reset_method sysfs node triggers this probe
path, which seems to propagate the vulnerability to this new execution path.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783044582.git.nicolinc@nvidia.com?part=2