[PATCH v2 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling

[PATCH v2 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling

[Priyank Rathod]

When firmware reports PCIe Advanced Error Reporting (AER) events via ACPI
APEI GHES (ghes_handle_aer()), it allocates a snapshot buffer from
ghes_estatus_pool to store the aer_capability_regs registers before
enqueuing the error record into aer_recover_ring.

If the error record cannot be enqueued or if the dequeued record cannot be
processed, the pool memory allocated for aer_capability_regs must be released.
Otherwise, unhandled errors silently leak memory in ghes_estatus_pool,
eventually leading to pool exhaustion and breaking system-wide GHES hardware
error reporting.

This series fixes two memory leaks in PCIe AER error handling:

Patch 1: Fixes a memory leak in aer_recover_queue() when kfifo_in_spinlocked()
         fails due to a buffer overflow (aer_recover_ring capacity of 16 full).
         The rejected entry is immediately freed via ghes_estatus_pool_region_free().

Patch 2: Fixes a memory leak in aer_recover_work_func() when a dequeued entry
         cannot be mapped to an active PCI device (pdev is NULL). Refactors the
         loop so ghes_estatus_pool_region_free() is called unconditionally for
         every dequeued item.

Changes in v2:
  - Refactored aer_recover_work_func() to ensure ghes_estatus_pool_region_free()
    is called unconditionally for every dequeued item.
  - Added Patch 1 to fix related memory leak in aer_recover_queue() on kfifo
    buffer overflow.
  - Updated commit messages with detailed pool allocation and lifecycle 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 v2:
- Refactored aer_recover_work_func() to ensure ghes_estatus_pool_region_free()
  is called unconditionally for every dequeued record.
- Added Patch 1 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 v2 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 v2 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 v2 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-v2-0-fd199b0171fd@google.com?part=2

Re: [PATCH v2 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 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Preexisting memory leak of `aer_regs` in `aer_recover_work_func()` when the associated PCI device is not found.
- [High] Out-of-bounds memory read and ABI layout mismatch when copying and parsing CPER AER payload.
--

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

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

When ACPI APEI/GHES processes PCIe AER error records, it allocates memory
for aer_capability_regs from ghes_estatus_pool. If kfifo_in_spinlocked()
fails due to a buffer overflow, the allocated memory is not freed. This
commit fixes the leak by calling ghes_estatus_pool_region_free() on the
error path.

> 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 isn't a bug introduced by this patch, but while you are fixing memory
leaks of aer_regs from ghes_estatus_pool, there appears to be another
pre-existing leak in the worker function aer_recover_work_func().

If pci_get_domain_bus_and_slot() fails to find the PCI device, it executes
a continue statement:

drivers/pci/pcie/aer.c:aer_recover_work_func() {
        ...
		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",
					   entry.domain, entry.bus,
					   PCI_SLOT(entry.devfn),
					   PCI_FUNC(entry.devfn));
			continue;
		}
        ...
        ghes_estatus_pool_region_free((unsigned long)entry.regs,
					    sizeof(struct aer_capability_regs));
}

Does this continue permanently leak entry.regs by skipping the
ghes_estatus_pool_region_free() call at the bottom of the loop?

[Severity: High]
This is a pre-existing issue, but since this patch interacts with
struct aer_capability_regs allocation, I noticed a potential struct layout
mismatch that could lead to an out-of-bounds read and corrupted hardware
error log parsing.

In drivers/acpi/apei/ghes.c:ghes_handle_aer(), the CPER payload is copied
using the size of the software structure:

        memcpy(aer_info, pcie_err->aer_info, sizeof(struct aer_capability_regs));

However, in include/linux/cper.h, pcie_err->aer_info is strictly a 96-byte
array. struct aer_capability_regs in include/linux/aer.h incorporates
struct pcie_tlp_log (60 bytes), expanding the struct size to 100 bytes and
shifting subsequent hardware fields (such as root_command).

Does this memcpy read 4 bytes past the end of the GHES payload? When
pci_print_aer() subsequently calls pcie_print_tlp_log(), could it read the
hardware Root Error Command register as if it were a TLP prefix, outputting
garbage data?

>  }
>  EXPORT_SYMBOL_GPL(aer_recover_queue);
>  #endif

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