[PATCH v3 0/2] (no cover subject)

[PATCH v3 0/2] (no cover subject)

[Priyank Rathod]

Changes in v3:
- Updated cover letter and auto-populated maintainer To/Cc trailers.
- Updated commit formatting per checkpatch guidelines.
- Link to v2: https://lore.kernel.org/r/20260803-b4-fix-aer-memleaks-v2-0-d2be03d9864b@google.com

PCI/AER: Fix ghes_estatus_pool memory leaks in error handling

This series addresses two memory leaks in PCIe Advanced Error Reporting (AER)
handling where memory snapshot buffers allocated from ghes_estatus_pool
are not released back to the pool.

When firmware reports PCIe errors via ACPI APEI GHES (ghes_handle_aer()), it
allocates a buffer from ghes_estatus_pool for aer_capability_regs and
enqueues it into aer_recover_ring. If errors are dropped or skipped, these
buffers must be freed to prevent ghes_estatus_pool memory exhaustion.

Patch 1: Fixes memory leak in aer_recover_queue() when kfifo_in_spinlocked()
         fails due to a buffer overflow (ring capacity of 16 entries full).
Patch 2: Fixes memory leak in aer_recover_work_func() when a dequeued entry
         cannot be mapped to a PCI device (pdev is NULL).

Changes in v2:
  - Refactored aer_recover_work_func() to ensure ghes_estatus_pool_region_free()
    is called unconditionally for every dequeued record.
  - Added Patch 2 to fix related memory leak in aer_recover_queue() on
    kfifo buffer overflow.
  - Updated commit messages with detailed pool lifetime explanations.

Priyank Rathod (2):
  PCI/AER: Fix memory leak in aer_recover_queue() on kfifo buffer overflow
  PCI/AER: Fix memory leak in aer_recover_work_func() when pci_dev is missing

 drivers/pci/pcie/aer.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
---
Changes in v3:
- Updated cover letter and auto-populated maintainer To/Cc trailers.
- Updated commit formatting per checkpatch guidelines.
- Link to v2: https://lore.kernel.org/r/20260803-b4-fix-aer-memleaks-v2-0-d2be03d9864b@google.com

Changes in v2:
- Refactored aer_recover_work_func() to ensure ghes_estatus_pool_region_free()
  is called unconditionally for every dequeued record.
- Added Patch 2 to fix related memory leak in aer_recover_queue() on
  kfifo buffer overflow.
- Link to v1: https://lore.kernel.org/r/20260803183853.432459-2-rathodpriyank@google.com

---
Priyank Rathod (2):
      PCI/AER: Fix memory leak in aer_recover_queue() on kfifo buffer overflow
      PCI/AER: Fix memory leak in aer_recover_work_func() when pci_dev is missing

 drivers/pci/pcie/aer.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)
---
base-commit: 248951ddc14de84de3910f9b13f51491a8cd91df
change-id: 20260803-b4-fix-aer-memleaks-524a1bd5e888

Best regards,
-- 
Priyank Rathod <rathodpriyank@google.com>

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

[Priyank Rathod]

When ACPI APEI/GHES processes PCIe AER error records, it allocates memory
for aer_capability_regs (aer_regs) from ghes_estatus_pool and passes it
to aer_recover_queue() to be enqueued into aer_recover_ring.

If kfifo_in_spinlocked() fails due to a buffer overflow,
aer_recover_queue() logged an error message but returned without freeing
aer_regs. Because the entry was rejected and never inserted into the
queue, aer_recover_work_func() could never dequeue or free it, leaking
the allocated ghes_estatus_pool memory.

Free aer_regs via ghes_estatus_pool_region_free() when
kfifo_in_spinlocked() fails on buffer overflow.

Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
---
 drivers/pci/pcie/aer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index c4fd9c0b2a54..967cde9dd519 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));
+	}
 }
 EXPORT_SYMBOL_GPL(aer_recover_queue);
 #endif

-- 
2.55.0.571.g244d577d93-goog

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

[Priyank Rathod]

When ACPI APEI/GHES processes PCIe AER error records, it allocates memory
for aer_capability_regs (entry.regs) from ghes_estatus_pool and queues
the entry into aer_recover_ring.

In aer_recover_work_func(), items are popped from aer_recover_ring via
kfifo_get(). If pci_get_domain_bus_and_slot() fails to find a matching
pci_dev, the code previously executed 'continue', bypassing the call to
ghes_estatus_pool_region_free(). As a result, the memory allocated for
entry.regs from ghes_estatus_pool was leaked.

Refactor aer_recover_work_func() to ensure ghes_estatus_pool_region_free()
is called unconditionally for every dequeued entry, releasing the pool
memory even when pci_dev is missing.

Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
---
 drivers/pci/pcie/aer.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 967cde9dd519..9683cdef649b 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1223,14 +1223,13 @@ static void aer_recover_work_func(struct work_struct *work)
 	while (kfifo_get(&aer_recover_ring, &entry)) {
 		pdev = pci_get_domain_bus_and_slot(entry.domain, entry.bus,
 						   entry.devfn);
-		if (!pdev) {
+		if (!pdev)
 			pr_err_ratelimited("%04x:%02x:%02x.%x: no pci_dev found\n",
 					   entry.domain, entry.bus,
 					   PCI_SLOT(entry.devfn),
 					   PCI_FUNC(entry.devfn));
-			continue;
-		}
-		pci_print_aer(pdev, entry.severity, entry.regs);
+		else
+			pci_print_aer(pdev, entry.severity, entry.regs);
 
 		/*
 		 * Memory for aer_capability_regs(entry.regs) is being
@@ -1242,13 +1241,15 @@ static void aer_recover_work_func(struct work_struct *work)
 		ghes_estatus_pool_region_free((unsigned long)entry.regs,
 					    sizeof(struct aer_capability_regs));
 
-		if (entry.severity == AER_NONFATAL)
-			pcie_do_recovery(pdev, pci_channel_io_normal,
-					 aer_root_reset);
-		else if (entry.severity == AER_FATAL)
-			pcie_do_recovery(pdev, pci_channel_io_frozen,
-					 aer_root_reset);
-		pci_dev_put(pdev);
+		if (pdev) {
+			if (entry.severity == AER_NONFATAL)
+				pcie_do_recovery(pdev, pci_channel_io_normal,
+						 aer_root_reset);
+			else if (entry.severity == AER_FATAL)
+				pcie_do_recovery(pdev, pci_channel_io_frozen,
+						 aer_root_reset);
+			pci_dev_put(pdev);
+		}
 	}
 }
 

-- 
2.55.0.571.g244d577d93-goog

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

[sashiko-bot]

> When ACPI APEI/GHES processes PCIe AER error records, it allocates memory
> for aer_capability_regs (entry.regs) from ghes_estatus_pool and queues
> the entry into aer_recover_ring.
> 
> In aer_recover_work_func(), items are popped from aer_recover_ring via
> kfifo_get(). If pci_get_domain_bus_and_slot() fails to find a matching
> pci_dev, the code previously executed 'continue', bypassing the call to
> ghes_estatus_pool_region_free(). As a result, the memory allocated for
> entry.regs from ghes_estatus_pool was leaked.
> 
> Refactor aer_recover_work_func() to ensure ghes_estatus_pool_region_free()
> is called unconditionally for every dequeued entry, releasing the pool
> memory even when pci_dev is missing.
> 
> Signed-off-by: Priyank Rathod <rathodpriyank@google.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

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

[sashiko-bot]

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