Re: [PATCH] PCI/AER: Fix memory leak in aer_recover_work_func() when pci_dev is missing

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

Pre-existing issues:
- [High] Memory allocated from `ghes_estatus_pool` for `aer_regs` is leaked=
 in `aer_recover_queue()` when `aer_recover_ring` kfifo is full.
--

--- Patch [1]: [PATCH] PCI/AER: Fix memory leak in aer_recover_work_func() =
when pci_dev is missing ---
commit 6bb7331494c5c24e65c258479be47a6a7424c240
Author: Priyank Rathod <[email protected]>

PCI/AER: Fix memory leak in aer_recover_work_func() when pci_dev is missing

This commit fixes a memory leak in aer_recover_work_func() by ensuring
that memory allocated for entry.regs is freed via
ghes_estatus_pool_region_free() unconditionally, even when
pci_get_domain_bus_and_slot() fails to find a matching PCI device.

> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index c4fd9c0b2a548..555f39b60b14d 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c

[ ... ]

> @@ -1242,13 +1241,15 @@ static void aer_recover_work_func(struct work_str=
uct *work)
>  		ghes_estatus_pool_region_free((unsigned long)entry.regs,
>  					    sizeof(struct aer_capability_regs));
> =20
> -		if (entry.severity =3D=3D AER_NONFATAL)
> -			pcie_do_recovery(pdev, pci_channel_io_normal,
> -					 aer_root_reset);
> -		else if (entry.severity =3D=3D AER_FATAL)
> -			pcie_do_recovery(pdev, pci_channel_io_frozen,
> -					 aer_root_reset);
> -		pci_dev_put(pdev);
> +		if (pdev) {
> +			if (entry.severity =3D=3D AER_NONFATAL)
> +				pcie_do_recovery(pdev, pci_channel_io_normal,
> +						 aer_root_reset);
> +			else if (entry.severity =3D=3D AER_FATAL)
> +				pcie_do_recovery(pdev, pci_channel_io_frozen,
> +						 aer_root_reset);
> +			pci_dev_put(pdev);
> +		}
>  	}
>  }

[Severity: High]
This is a pre-existing issue, but while reviewing this fix for the memory
leak in the consumer (aer_recover_work_func), I noticed a similar leak
could occur in the producer (aer_recover_queue).

In drivers/acpi/apei/ghes.c:ghes_handle_aer(), aer_info is allocated from
the ghes_estatus_pool:

	aer_info =3D (void *)gen_pool_alloc(ghes_estatus_pool,
					  sizeof(struct aer_capability_regs));
	/* ... */
	aer_recover_queue(pcie_err->device_id.segment,
			  pcie_err->device_id.bus,
			  devfn, aer_severity,
			  (struct aer_capability_regs *)
			  aer_info);

If the aer_recover_ring kfifo is full (e.g., during an AER storm with more
than 16 errors), kfifo_in_spinlocked() fails in aer_recover_queue():

drivers/pci/pcie/aer.c:aer_recover_queue() {
    ...
	if (kfifo_in_spinlocked(&aer_recover_ring, &entry, 1,
				 &aer_recover_ring_lock))
		schedule_work(&aer_recover_work);
	else
		pr_err("buffer overflow in recovery for %04x:%02x:%02x.%x\n",
		       domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
}

Since aer_recover_queue() returns void, the caller ghes_handle_aer() does
not know if the queueing failed. The else branch logs a buffer overflow but
does not call ghes_estatus_pool_region_free() to release the memory.

Could this exhaust the finite ghes_estatus_pool during an AER storm,
preventing the system from handling future hardware errors like fatal MCEs?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803183853.4324=
[email protected]?part=3D1