[PATCH v2] xen/pcifront: Fix PCI device reference leak in AER handling

[PATCH v2] xen/pcifront: Fix PCI device reference leak in AER handling

[Ruoyu Wang]

pci_get_domain_bus_and_slot() increments the reference count of the
returned PCI device. pcifront_common_process() drops that reference only
when the device or its driver is missing. All paths for a bound device
either return directly after invoking an error recovery callback or fall
through without calling pci_dev_put(). Consequently, each AER request for
a bound device leaks a reference and can keep the device allocated after
removal.

Declare the looked-up device with __free(pci_dev_put), so every return
path releases the reference after callback dispatch. This keeps the
device alive while its callback runs and balances the lookup without
restructuring the callback returns.

This issue was found by a static analysis checker and confirmed by manual
source review.

Fixes: 956a9202cd12 ("xen-pcifront: Xen PCI frontend driver.")
Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
Changes in v2:
- Use __free(pci_dev_put) instead of restructuring the callback returns.

Link: https://lore.kernel.org/r/20260813153138.3953222-1-ruoyuw560@gmail.com/
---
 drivers/pci/xen-pcifront.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index cffc32d660327..0dea8a69fa36c 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -579,16 +579,15 @@ static pci_ers_result_t pcifront_common_process(int cmd,
 	int bus = pdev->sh_info->aer_op.bus;
 	int devfn = pdev->sh_info->aer_op.devfn;
 	int domain = pdev->sh_info->aer_op.domain;
-	struct pci_dev *pcidev;
+	struct pci_dev *pcidev __free(pci_dev_put) =
+		pci_get_domain_bus_and_slot(domain, bus, devfn);
 
 	dev_dbg(&pdev->xdev->dev,
 		"pcifront AER process: cmd %x (bus:%x, devfn%x)",
 		cmd, bus, devfn);
 
-	pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
 	if (!pcidev || !pcidev->dev.driver) {
 		dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
-		pci_dev_put(pcidev);
 		return PCI_ERS_RESULT_NONE;
 	}
 	pdrv = to_pci_driver(pcidev->dev.driver);
-- 
2.51.0

Re: [PATCH v2] xen/pcifront: Fix PCI device reference leak in AER handling

[sashiko-bot]

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

Pre-existing issues:
- [High] Optional AER error handler callbacks are invoked unconditionally without being checked for existence, leading to a NULL pointer dereference.
- [High] Unsafe lockless access to `pcidev->dev.driver` leads to a TOCTOU race condition and Use-After-Free against driver unbinding.
--

--- Patch [1]: [PATCH v2] xen/pcifront: Fix PCI device reference leak in AER handling ---
commit e7640edce237b4072d2fc35f47ae61c01c1ebc16
Author: Ruoyu Wang <ruoyuw560@gmail.com>
Subject: xen/pcifront: Fix PCI device reference leak in AER handling

This commit fixes a PCI device reference leak in the Xen PCI frontend
driver's AER handling. It declares the looked-up device with the
__free(pci_dev_put) cleanup attribute. This ensures the reference is properly
dropped on all return paths.

Link: https://lore.kernel.org/r/20260813153138.3953222-1-ruoyuw560@gmail.com/

> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index cffc32d660327..0dea8a69fa36c 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -579,16 +579,15 @@ static pci_ers_result_t pcifront_common_process(int cmd,
>  	int bus = pdev->sh_info->aer_op.bus;
>  	int devfn = pdev->sh_info->aer_op.devfn;
>  	int domain = pdev->sh_info->aer_op.domain;
> -	struct pci_dev *pcidev;
> +	struct pci_dev *pcidev __free(pci_dev_put) =
> +		pci_get_domain_bus_and_slot(domain, bus, devfn);
>  
>  	dev_dbg(&pdev->xdev->dev,
>  		"pcifront AER process: cmd %x (bus:%x, devfn%x)",
>  		cmd, bus, devfn);
>  
> -	pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
>  	if (!pcidev || !pcidev->dev.driver) {
>  		dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
> -		pci_dev_put(pcidev);
>  		return PCI_ERS_RESULT_NONE;
>  	}
>  	pdrv = to_pci_driver(pcidev->dev.driver);

[Severity: High]
This isn't a bug introduced by this patch, but does accessing
pcidev->dev.driver locklessly here introduce a race condition?

If a device unbind races with an incoming AER event, to_pci_driver(__drv)
might evaluate pcidev->dev.driver to NULL on its second access, or the
driver could be unloaded before callbacks are invoked. Standard PCI AER
handlers usually hold device_lock(&dev->dev) to serialize against driver
binding and unbinding.

[Severity: High]
This is a pre-existing issue, but further down in pcifront_common_process(),
does the code invoke optional AER callbacks without checking if they exist?

While the error_detected callback is verified, optional callbacks appear to
be invoked unconditionally:

    case XEN_PCI_OP_aer_mmio:
        return pdrv->err_handler->mmio_enabled(pcidev);
    case XEN_PCI_OP_aer_slotreset:
        return pdrv->err_handler->slot_reset(pcidev);

If a bound driver lacks these optional callbacks, will processing these
commands from the backend result in a NULL pointer dereference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820135603.3901798-1-ruoyuw560@gmail.com?part=1

Re: [PATCH v2] xen/pcifront: Fix PCI device reference leak in AER handling

[Lukas Wunner]

On Thu, Aug 20, 2026 at 09:56:03PM +0800, Ruoyu Wang wrote:
> pci_get_domain_bus_and_slot() increments the reference count of the
> returned PCI device. pcifront_common_process() drops that reference only
> when the device or its driver is missing. All paths for a bound device
> either return directly after invoking an error recovery callback or fall
> through without calling pci_dev_put(). Consequently, each AER request for
> a bound device leaks a reference and can keep the device allocated after
> removal.
> 
> Declare the looked-up device with __free(pci_dev_put), so every return
> path releases the reference after callback dispatch. This keeps the
> device alive while its callback runs and balances the lookup without
> restructuring the callback returns.
> 
> This issue was found by a static analysis checker and confirmed by manual
> source review.
> 
> Fixes: 956a9202cd12 ("xen-pcifront: Xen PCI frontend driver.")
> Suggested-by: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>

Reviewed-by: Lukas Wunner <lukas@wunner.de>