Re: [PATCH v3 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:
- [Critical] Missing error cleanup in `pcibios_enable_device()` leads to permanent FMB memory leak, debugfs dentry leak, and a Critical Use-After-Free triggerable by unprivileged users.
- [High] Time-of-Check to Time-of-Use (TOCTOU) race condition in `pcibios_disable_device()` allows userspace to bypass device teardown and re-enable the PCI function measurement block (FMB) via debugfs, causing a memory leak.
--

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

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

This commit ensures that fmb_lock is held by pcibios_enable_device() and
pcibios_disable_device() when calling zpci_fmb_enable_device() or
zpci_fmb_disable_device(). It also asserts that the lock is held inside
those functions to prevent future race conditions.

Fixes: af0a8a8453f7 ("s390/pci: implement pcibios_add_device")
Fixes: 944239c59e93 ("s390/pci: implement pcibios_release_device")
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: Critical]
This is a pre-existing issue, but does this code leak resources and expose
a use-after-free regression if pci_enable_resources() fails?

If pci_enable_resources() returns an error, the function returns without
cleaning up the allocated zdev->fmb or the debugfs files created by
zpci_debug_init_device(). Since the PCI core considers the device enablement
to have failed, pcibios_disable_device() will never be called.

When the device is eventually hot-removed, zpci_release_device() frees zdev.
Because the debugfs files were never removed, they remain active with the
freed zdev as their private_data. An unprivileged userspace process can then
read the dangling 'statistics' file, invoking pci_perf_show() which
dereferences the freed zdev->fmb_lock, leading to 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);

[Severity: High]
This is a pre-existing issue, but does dropping the lock before destroying the
debugfs interface expose a time-of-check to time-of-use race?

If userspace writes '1' to the debugfs 'statistics' file right after the lock
is dropped:

CPU1                                     CPU2
pcibios_disable_device()                 pci_perf_seq_write()
  mutex_unlock(&zdev->fmb_lock);
                                           mutex_lock(&zdev->fmb_lock);
                                           zpci_fmb_enable_device();
                                           mutex_unlock(&zdev->fmb_lock);
  zpci_debug_exit_device();

This allows the hardware function measurement block (FMB) to be re-enabled and
new memory allocated. When zpci_debug_exit_device() finishes, it silently
succeeds, leaving the newly allocated zdev->fmb and hardware state active.

When the device is later released, zpci_release_device() frees zdev but does
not free zdev->fmb, causing a permanent memory leak and leaving the hardware
FMB continuously performing DMA to the leaked slab memory.

>  	zpci_debug_exit_device(zdev);
>  }

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