Re: [PATCH v19 1/4] 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:

New issues:
- [High] Global enforcement of per_func_slot on S390 breaks standard generic PCI hotplug (e.g., pciehp) slot matching.

Pre-existing issues:
- [High] Pre-existing Race Conditions and Use-After-Free in PCI slot lifecycle and dev->slot management.
--

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

This commit adds a per_func_slot flag to struct pci_slot to support
resetting individual PCI functions on s390 systems. It modifies the slot
matching and reset logic to account for this new flag, and adjusts the
slot number type to handle placeholder 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))

[Severity: High]
This is a pre-existing issue, but does reading dev->slot->per_func_slot here
expose a use-after-free vulnerability?

In pci_dev_reset_slot_function(), dev->slot is dereferenced locklessly without
holding pci_bus_sem. If a concurrent hotplug removal triggers
pci_slot_release(), the slot could be freed.

Furthermore, pci_slot_release() iterates over bus->devices to clear dev->slot,
but this can miss devices currently undergoing probing that haven't been added
to the bus list yet, leaving a dangling pointer.

>  		return -ENOTTY;
>  
>  	return pci_reset_hotplug_slot(dev->slot->hotplug, probe);

[ ... ]

> 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;
> +

[Severity: High]
Does this logic break generic hotplug drivers like pciehp on s390 systems?

Because pci_slot_enabled_per_func() unconditionally returns true for
CONFIG_S390, per_func_slot is enabled globally. When generic hotplug drivers
create slots using PCI_SLOT_ALL_DEVICES (0xfeff), slot->number is 0xfeff.

If slot->per_func_slot is true, this evaluates dev->devfn == 0xfeff before
checking for the PCI_SLOT_ALL_DEVICES wildcard. Since devfn is an 8-bit value,
this always evaluates to false, preventing downstream devices from matching
their hotplug slots.

> +	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;
> +}

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