Re: [PATCH v5 1/4] s390/pci: Hold fmb_lock when enabling or disabling PCI devices

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing cleanup in pcibios_enable_device() on pci_enable_resources() failure leads to FMB memory leak and Use-After-Free.
- [Medium] Race condition between pcibios_disable_device() and pci_perf_seq_write() allows FMB to be re-enabled on a disabled device, causing a memory leak.
--

commit 2e2fca3d1e089622d5f93c6cb6a886a708c7c22d
Author: Omar Elghoul <oelghoul@linux.ibm.com>

s390/pci: Hold fmb_lock when enabling or disabling PCI devices

This commit ensures fmb_lock is held in pcibios_enable_device() and
pcibios_disable_device() before calling the zpci fmb enable/disable
functions. It also adds lockdep assertions to prevent future races.

Cc: stable@vger.kernel.org

> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 39bd2adfc240f..2910d4038d39c 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c

[ ... ]

> @@ -639,7 +643,9 @@ int pcibios_enable_device(struct pci_dev *pdev, int mask)
>  	struct zpci_dev *zdev = to_zpci(pdev);
>  
>  	zpci_debug_init_device(zdev, dev_name(&pdev->dev));
> +	mutex_lock(&zdev->fmb_lock);
>  	zpci_fmb_enable_device(zdev);
> +	mutex_unlock(&zdev->fmb_lock);
>  
>  	return pci_enable_resources(pdev, mask);
>  }

[Severity: High]
This is a pre-existing issue, but does this lack of cleanup on failure lead
to a use-after-free?

If pci_enable_resources() fails, the function returns an error without rolling
back the debugfs initialization or the FMB allocations. Because the PCI core
considers the device disabled, pcibios_disable_device() is never called to
clean these up.

When the device is eventually released, pcibios_release_device() will drop
the zdev references without destroying the debugfs entries:

arch/s390/pci/pci.c:pcibios_release_device() {
    ...
    zpci_unmap_resources(pdev);
    zpci_zdev_put(zdev);
}

Can a subsequent userspace access to the orphaned debugfs files dereference
the freed zdev pointer and trigger a use-after-free?

> @@ -648,7 +654,9 @@ void pcibios_disable_device(struct pci_dev *pdev)
>  {
>  	struct zpci_dev *zdev = to_zpci(pdev);
>  
> +	mutex_lock(&zdev->fmb_lock);
>  	zpci_fmb_disable_device(zdev);
> +	mutex_unlock(&zdev->fmb_lock);
>  	zpci_debug_exit_device(zdev);
>  }

[Severity: Medium]
This isn't a bug introduced by this patch, but does unlocking fmb_lock before
zpci_debug_exit_device() create a race condition with debugfs writes?

A concurrent write to the debugfs statistics file could potentially execute in
the window between mutex_unlock() and zpci_debug_exit_device() removing the
files:

arch/s390/pci/pci_debug.c:pci_perf_seq_write() {
    ...
    mutex_lock(&zdev->fmb_lock);
    switch (val) {
    ...
    case 1:
        rc = zpci_fmb_enable_device(zdev);
        break;
    }
    mutex_unlock(&zdev->fmb_lock);
}

If pci_perf_seq_write() acquires the lock in this window, could it successfully
re-enable the FMB in hardware right before zpci_debug_exit_device() destroys
the debug entries, leaving the FMB enabled but permanently leaking the memory
upon device release?

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