Re: [PATCH v18 1/3] 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but a Use-After-Free (UAF) vulnerability exists due to lockless access to `dev->slot` in `pci_dev_reset_slot_function()`. The function reads `dev->slot` to check `!dev->slot`, and subsequently dereferences it (`dev->slot->per_func_slot` and `dev->slot->hotplug`). However, it does not hold `pci_slot_mutex` or `pci_bus_sem`. Concurrently, if a hotplug service driver (like `pciehp`) is unbound, it triggers `pci_destroy_slot()` which drops the refcount and calls `pci_slot_release()`. This sets `dev->slot = NULL` and frees the slot memory. A race condition allows the reset function to read the slot pointer before it is cleared and then dereference freed memory.
--

commit ce8f71bdb64d4b651c9d934da7ecbb2d2811bf71
Author: Farhan Ali <alifm@linux.ibm.com>

PCI: Allow per function PCI slots to fix slot reset on s390

This commit introduces a flag in struct pci_slot to allow per-function
PCI slots for hypervisor-managed functions. It also modifies the slot
reset function to account for this new flag and handle multi-function
devices appropriately on s390 systems.

[ ... ]

> 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))
>  		return -ENOTTY;
>  
>  	return pci_reset_hotplug_slot(dev->slot->hotplug, probe);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for a concurrent
hotplug driver unbind to cause a use-after-free here?

Looking at pci_dev_reset_slot_function(), dev->slot is checked and then
dereferenced multiple times locklessly:

    if (dev->subordinate || !dev->slot ||
        dev->dev_flags & PCI_DEV_FLAGS_NO_BUS_RESET ||
        (dev->multifunction && !dev->slot->per_func_slot))
            return -ENOTTY;

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

If a hotplug service driver like pciehp is unbound concurrently, it triggers
pci_destroy_slot() which drops the refcount and eventually calls
pci_slot_release(). This clears dev->slot and frees the memory:

pci_slot_release() {
    ...
    down_read(&pci_bus_sem);
    list_for_each_entry(dev, &slot->bus->devices, bus_list)
        if (pci_dev_matches_slot(dev, slot))
            dev->slot = NULL;
    up_read(&pci_bus_sem);
    ...
    kfree(slot);
}

Could the reset function read dev->slot, get interrupted by the slot release,
and then dereference the freed memory?

[ ... ]

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