Re: [PATCH v2 1/2] s390/pci: Fix missing pci_dev_put() in zpci_report_status()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Lockless TOCTOU and Use-After-Free when accessing `pdev->dev.driver` and `driver->name`.
--

--- Patch [1]: [PATCH v2 1/2] s390/pci: Fix missing pci_dev_put() in zpci_report_status() ---
commit 81196a1544a6f30ec9381d6aeaf28e9992efad5f
Author: Niklas Schnelle <schnelle@linux.ibm.com>

    s390/pci: Fix missing pci_dev_put() in zpci_report_status()
    
    In zpci_report_status() a reference to the struct pci_dev associated
    with the struct zpci_dev being reported about is acquired using
    pci_get_slot(). This reference needs to be dropped with pci_dev_put()
    but this call is missing thus leaking the reference. On subsequent hot
    unplug this will cause the struct pci_dev to not be released leaking
    memory and potentially prevent reattach.

> diff --git a/arch/s390/pci/pci_report.c b/arch/s390/pci/pci_report.c
> index 7030f7052926a..76b8afc146fea 100644
> --- a/arch/s390/pci/pci_report.c
> +++ b/arch/s390/pci/pci_report.c
> @@ -151,6 +151,7 @@ int zpci_report_status(struct zpci_dev *zdev, const char *operation, const char
>  	else
>  		pr_info("Reported PCI device status\n");
>  
> +	pci_dev_put(pdev);

[Severity: High]
This isn't a bug introduced by this patch, but while we are working with
pdev in zpci_report_status(), is it safe that earlier in this function
we access pdev->dev.driver locklessly?

arch/s390/pci/pci_report.c:zpci_report_status() {
...
	if (pdev)
		driver = to_pci_driver(pdev->dev.driver);
...
	buf += scnprintf(buf, end - buf, "driver: %s\n", (driver) ? driver->name : "n/a");
...
}

If a concurrent driver unbind (e.g., via sysfs) clears pdev->dev.driver while
zpci_report_status() is executing, the to_pci_driver() macro could read
it during a race. The to_pci_driver() macro evaluates its argument twice.
If pdev->dev.driver becomes NULL between these evaluations, it subtracts
the offset from NULL, yielding a large negative pointer.

Can this dereference crash the kernel if driver is a negative pointer from
the race?

Additionally, even if the macro evaluates safely, accessing driver->name
without holding the device lock or preventing module unload could result
in a use-after-free if the driver module is concurrently unloaded.

Could this be triggered by an administrator unbinding a PCI driver
concurrently with a PCI error recovery event?

>  	free_page((unsigned long)report);
>  
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-fix_zpci_report_status_pdev_leak-v2-0-da0c4aa7c0ca@linux.ibm.com?part=1