Re: [PATCH v15 1/7] PCI: Allow per function PCI slots to fix slot reset on s390

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Global enablement of `per_func_slot` on S390 breaks standard PCIe hotplug matching and hardware association.
- [High] Lockless read of `dev->slot` in `pci_dev_reset_slot_function()` leads to a Time-Of-Check to Time-Of-Use (TOCTOU) Use-After-Free.
--

commit e4e35148b2442f92d3db17a20a087722c65c043d
Author: Farhan Ali <alifm@linux.ibm.com>
PCI: Allow per function PCI slots to fix slot reset on s390

This commit introduces a per-function PCI slot flag to fix slot resets for
multifunction devices on s390 systems. It updates the PCI slot number field
to a 16-bit integer to accommodate special placeholder and all-devices values.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8f7cfcc000901..d0c9f0166af5c 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
[ ... ]
> @@ -4865,8 +4865,9 @@ static int pci_reset_hotplug_slot(struct hotplug_slot *hotplug, bool probe)
>  
>  static int pci_dev_reset_slot_function(struct pci_dev *dev, bool probe)
>  {
> -	if (dev->multifunction || dev->subordinate || !dev->slot ||
> -	    dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET)
> +	if (dev->subordinate || !dev->slot ||
> +	    dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET ||
> +	    (dev->multifunction && !dev->slot->per_func_slot))

Does this code introduce a time-of-check to time-of-use use-after-free
regression?

When dev->slot is accessed locklessly, it could be freed concurrently if a
hotplug driver module unload invokes pci_destroy_slot() and
pci_slot_release(), which clears dev->slot to NULL under pci_bus_sem and
frees the slot.

Because this function does not hold pci_bus_sem or elevate the slot's
reference count, adding another lockless dereference to
dev->slot->per_func_slot right after the !dev->slot check could access
freed memory if the slot is destroyed in between.

> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
> index 6d5cd37bfb1e7..894d6213ed306 100644
> --- a/drivers/pci/slot.c
> +++ b/drivers/pci/slot.c
[ ... ]
> @@ -72,6 +72,23 @@ static ssize_t cur_speed_read_file(struct pci_slot *slot, char *buf)
>  	return bus_speed_read(slot->bus->cur_bus_speed, buf);
>  }
>  
> +static bool pci_dev_matches_slot(struct pci_dev *dev, struct pci_slot *slot)
> +{
> +	if (slot->per_func_slot)
> +		return dev->devfn == slot->number;
> +
> +	return slot->number == PCI_SLOT_ALL_DEVICES ||
> +		PCI_SLOT(dev->devfn) == slot->number;
> +}
> +
> +static bool pci_slot_enabled_per_func(void)
> +{
> +	if (IS_ENABLED(CONFIG_S390))
> +		return true;
> +
> +	return false;
> +}

Does this global enablement cause a regression for standard PCIe hotplug
matching on s390 systems?

By unconditionally returning true on s390, slot->per_func_slot is globally
set to 1 for all hotplug slots created. When per_func_slot is true,
pci_dev_matches_slot() prioritizes an exact match of
dev->devfn == slot->number.

However, standard hotplug drivers like pciehp initialize slots using
PCI_SLOT_ALL_DEVICES (now 0xfeff). Since a device's devfn is an 8-bit value,
this will never match, which might completely break pciehp topology on s390
environments such as KVM guests.

Additionally, if generic drivers like shpchp register slots using just the
device number (for example, 5), the strict match will compare devfn to 5
(device 0, function 5) instead of the correct device 5, function 0
(devfn 40). Could this lead to failed associations or cross-device resets?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505200510.2954-1-alifm@linux.ibm.com?part=1