Re: [PATCH] PCI/AER: Fix struct pci_dev reference leak in aer_process_err_devices()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Lockless access to `dev->driver` in `pci_aer_handle_error` can lead to use-after-free if the driver is concurrently unbound.
--

commit 9885b6e8d159d70ef8c2c474b114ae69fb1805af
Author: Priyank Rathod <rathodpriyank@google.com>

PCI/AER: Fix struct pci_dev reference leak in aer_process_err_devices()

This commit fixes a reference leak on struct pci_dev that occurs when a device
without the AER extended capability is added as an error source. It does this
by decoupling device reference release from handle_error_source() and calling
pci_dev_put() unconditionally in aer_process_err_devices().

> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index c4fd9c0b2a548..2401b3a18f07d 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1197,7 +1197,6 @@ static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
>  {
>  	cxl_rch_handle_error(dev, info);
>  	pci_aer_handle_error(dev, info);

[Severity: High]
This isn't a bug introduced by this patch, but does pci_aer_handle_error()
safely access dev->driver?

Looking at pci_aer_handle_error() in drivers/pci/pcie/aer.c, when handling
correctable errors, it reads dev->driver into a local pointer without
holding the device lock:

static void pci_aer_handle_error(struct pci_dev *dev, struct aer_err_info *info)
{
	...
	if (info->severity == AER_CORRECTABLE) {
		...
		if (pcie_aer_is_native(dev)) {
			struct pci_driver *pdrv = dev->driver;

			if (pdrv && pdrv->err_handler &&
			    pdrv->err_handler->cor_error_detected)
				pdrv->err_handler->cor_error_detected(dev);
			...
		}
	}
	...
}

Since pci_aer_handle_error() executes in a threaded IRQ context (via
aer_process_err_devices()), could a concurrent sysfs driver unbind or module
unload clear dev->driver and free the driver module immediately after the
pointer is read?

This contrasts with pcie_do_recovery() and cxl_rch_handle_error_iter(), which
appear to correctly hold the device lock (device_lock(&dev->dev)) before
accessing dev->driver.

> -	pci_dev_put(dev);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830-fix-aer-refcount-leak-v1-1-64e1013add12@google.com?part=1