Re: [PATCH v3 1/2] PCI/AER: Fix memory leak in aer_recover_queue() on kfifo buffer overflow

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing memory leak of `entry.regs` in `aer_recover_work_func()` when the target PCI device is not found.
--

--- Patch [1]: [PATCH v3 1/2] PCI/AER: Fix memory leak in aer_recover_queue() on kfifo buffer overflow ---
commit d5a765be7d938a7478de89568f1bd78dc7110e67
Author: Priyank Rathod <rathodpriyank@google.com>

PCI/AER: Fix memory leak in aer_recover_queue() on kfifo buffer overflow

This commit addresses a memory leak that occurs when ACPI APEI/GHES attempts
to enqueue a new AER error record but the kfifo buffer overflows. It ensures
that the dynamically allocated memory for aer_regs is properly freed using
ghes_estatus_pool_region_free() when kfifo_in_spinlocked() fails.

> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index c4fd9c0b2a548..967cde9dd5191 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1274,9 +1274,12 @@ void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
>  	if (kfifo_in_spinlocked(&aer_recover_ring, &entry, 1,
>  				 &aer_recover_ring_lock))
>  		schedule_work(&aer_recover_work);
> -	else
> +	else {
>  		pr_err("buffer overflow in recovery for %04x:%02x:%02x.%x\n",
>  		       domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> +		ghes_estatus_pool_region_free((unsigned long)aer_regs,
> +					    sizeof(struct aer_capability_regs));
> +	}

[Severity: High]
This is a pre-existing issue, but while reviewing this fix, I noticed
another potential memory leak of entry.regs in aer_recover_work_func().

When aer_recover_work_func() dequeues an entry, it attempts to find the pdev:

drivers/pci/pcie/aer.c:aer_recover_work_func() {
	...
	while (kfifo_get(&aer_recover_ring, &entry)) {
		pdev = pci_get_domain_bus_and_slot(entry.domain, entry.bus,
						   entry.devfn);
		if (!pdev) {
			pr_err_ratelimited("%04x:%02x:%02x.%x: no pci_dev found\n",
					   ...);
			continue;
		}
	...
}

If pdev is not found, the continue statement bypasses the subsequent call to
ghes_estatus_pool_region_free().

Does this code permanently leak the constrained ghes_estatus_pool memory for
entry.regs when a device is not enumerated or hot-removed?

>  }
>  EXPORT_SYMBOL_GPL(aer_recover_queue);
>  #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-b4-fix-aer-memleaks-v3-0-e87159611933@google.com?part=1