mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling
@ 2026-09-18 17:24 Priyank Rathod
  2026-09-18 17:24 ` [PATCH v4 1/2] PCI/AER: Fix memory leak in aer_recover_queue() on kfifo buffer overflow Priyank Rathod
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Priyank Rathod @ 2026-09-18 17:24 UTC (permalink / raw)
  To: Mahesh J Salgaonkar, Oliver O'Halloran, Bjorn Helgaas
  Cc: Lukas Wunner, linuxppc-dev, linux-pci, linux-kernel,
	Priyank Rathod, stable

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.

aer_recover_queue() returns void, so ghes_handle_aer() cannot release that
buffer itself; ownership is handed to the AER code, which until now freed
it only on the fully successful path. If the record cannot be enqueued, or
if a dequeued record cannot be mapped to a pci_dev, the allocation is
silently leaked. Under a sustained error storm this exhausts
ghes_estatus_pool, which then breaks GHES hardware error reporting
system-wide.

This series fixes both leak paths:

Patch 1: aer_recover_queue() when kfifo_in_spinlocked() fails because
         aer_recover_ring (capacity 16) is full. The rejected entry is
         freed immediately via ghes_estatus_pool_region_free().

Patch 2: aer_recover_work_func() when a dequeued entry cannot be mapped to
         an active PCI device (pdev is NULL). The loop is restructured so
         ghes_estatus_pool_region_free() runs unconditionally for every
         dequeued item.

Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
---
Changes in v4:
- Rebased onto v7.3-rc3+ (f259f446f519); applies cleanly to pci/next as
  well. No conflicts with the Advisory Non-Fatal Error support that landed
  in the meantime.
- Added missing Fixes: e2abc47a5a1a ("ACPI: APEI: Fix AER info corruption
  when error status data has multiple sections") and Cc: stable to both
  patches; that commit (v6.7-rc1) introduced the ghes_estatus_pool
  allocation whose ownership these paths drop.
- Patch 1: use braces on both arms of the if/else and fix the continuation
  alignment (checkpatch --strict).
- Both patches now build warning-free with W=1 and CONFIG_ACPI_APEI_PCIEAER=y
  (earlier revisions were only build-tested with APEI disabled, which
  compiles neither of the modified functions).
- Explained in both commit messages why the caller cannot free the buffer,
  and when the missing-pci_dev path is reachable.
- Cc: Lukas Wunner, who has been active in this code.
- Link to v3: https://lore.kernel.org/r/20260803-b4-fix-aer-memleaks-v3-1-e87159611933@google.com

Changes in v3:
- Resent to fix threading of the series.
- Link to v2: https://lore.kernel.org/r/20260803-b4-fix-aer-memleaks-v2-1-fd199b0171fd@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 | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)
---
base-commit: f259f446f5198d98e13756d2cd531812a0ad3064
change-id: 20260803-b4-fix-aer-memleaks-524a1bd5e888

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 1/2] PCI/AER: Fix memory leak in aer_recover_queue() on kfifo buffer overflow
  2026-09-18 17:24 [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling Priyank Rathod
@ 2026-09-18 17:24 ` Priyank Rathod
  2026-09-18 17:25 ` [PATCH v4 2/2] PCI/AER: Fix memory leak in aer_recover_work_func() when pci_dev is missing Priyank Rathod
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Priyank Rathod @ 2026-09-18 17:24 UTC (permalink / raw)
  To: Mahesh J Salgaonkar, Oliver O'Halloran, Bjorn Helgaas
  Cc: Lukas Wunner, linuxppc-dev, linux-pci, linux-kernel,
	Priyank Rathod, stable

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.

aer_recover_queue() returns void, so the caller ghes_handle_aer() cannot
free the buffer itself: ownership is transferred to the AER code, which
until now only released it on the success path.

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

Fixes: e2abc47a5a1a ("ACPI: APEI: Fix AER info corruption when error status data has multiple sections")
Cc: stable@vger.kernel.org
Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
---
 drivers/pci/pcie/aer.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index d8dcd238fda1..b013b853b555 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1415,11 +1415,14 @@ 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))
+				 &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.1082.g2b9226bbc0-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 2/2] PCI/AER: Fix memory leak in aer_recover_work_func() when pci_dev is missing
  2026-09-18 17:24 [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling Priyank Rathod
  2026-09-18 17:24 ` [PATCH v4 1/2] PCI/AER: Fix memory leak in aer_recover_queue() on kfifo buffer overflow Priyank Rathod
@ 2026-09-18 17:25 ` Priyank Rathod
  2026-09-18 18:15 ` [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling Priyank Rathod
  2026-09-25 18:24 ` Kuppuswamy Sathyanarayanan
  3 siblings, 0 replies; 5+ messages in thread
From: Priyank Rathod @ 2026-09-18 17:25 UTC (permalink / raw)
  To: Mahesh J Salgaonkar, Oliver O'Halloran, Bjorn Helgaas
  Cc: Lukas Wunner, linuxppc-dev, linux-pci, linux-kernel,
	Priyank Rathod, stable

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.

This is reachable whenever the device reported by firmware is not (or is
no longer) present in the PCI device tree, e.g. after hot-removal or when
firmware reports an error for a device the kernel never enumerated.

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.

Fixes: e2abc47a5a1a ("ACPI: APEI: Fix AER info corruption when error status data has multiple sections")
Cc: stable@vger.kernel.org
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 b013b853b555..a6600801af6e 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1366,14 +1366,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
@@ -1385,13 +1384,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.1082.g2b9226bbc0-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling
  2026-09-18 17:24 [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling Priyank Rathod
  2026-09-18 17:24 ` [PATCH v4 1/2] PCI/AER: Fix memory leak in aer_recover_queue() on kfifo buffer overflow Priyank Rathod
  2026-09-18 17:25 ` [PATCH v4 2/2] PCI/AER: Fix memory leak in aer_recover_work_func() when pci_dev is missing Priyank Rathod
@ 2026-09-18 18:15 ` Priyank Rathod
  2026-09-25 18:24 ` Kuppuswamy Sathyanarayanan
  3 siblings, 0 replies; 5+ messages in thread
From: Priyank Rathod @ 2026-09-18 18:15 UTC (permalink / raw)
  To: Mahesh J Salgaonkar, Oliver O'Halloran, Bjorn Helgaas
  Cc: Lukas Wunner, Kuppuswamy Sathyanarayanan, Jonathan Cameron,
	Ilpo Järvinen, Dave Jiang, linuxppc-dev, linux-pci,
	linux-kernel, Priyank Rathod

[+cc Kuppuswamy, Jonathan, Ilpo, Dave - you have reviewed most of the
 recent aer.c changes, so adding you here]

Hi all,

Adding the reviewers who have been active in drivers/pci/pcie/aer.c, as
this series has not had review feedback since v1 (3 Aug).

Short summary: ghes_handle_aer() hands a ghes_estatus_pool allocation to
aer_recover_queue(), which returns void, so ownership sits with the AER
code. Two paths drop it without freeing - kfifo overflow in
aer_recover_queue(), and a dequeued record with no matching pci_dev in
aer_recover_work_func(). Under a sustained error storm this drains the
pool, which then breaks GHES hardware error reporting system-wide.

v4 adds the Fixes: e2abc47a5a1a tag and Cc: stable that earlier
revisions were missing, and is rebased onto v7.3-rc3+ (applies cleanly
to pci/next as well).

Review feedback very welcome - happy to respin in whatever shape you
prefer. One open design question I would specifically like an opinion
on: patch 1 frees the buffer inside aer_recover_queue(), which bakes the
ghes_estatus_pool ownership assumption into an exported symbol. The
alternative is to make aer_recover_queue() return int and let
ghes_handle_aer() free its own allocation. I went with the former
because aer_recover_work_func() already frees unconditionally to the
pool, but I am happy to switch if you consider the exported-API
contract cleaner.

Thanks,
Priyank

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling
  2026-09-18 17:24 [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling Priyank Rathod
                   ` (2 preceding siblings ...)
  2026-09-18 18:15 ` [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling Priyank Rathod
@ 2026-09-25 18:24 ` Kuppuswamy Sathyanarayanan
  3 siblings, 0 replies; 5+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2026-09-25 18:24 UTC (permalink / raw)
  To: Priyank Rathod, Mahesh J Salgaonkar, Oliver O'Halloran,
	Bjorn Helgaas
  Cc: Lukas Wunner, linuxppc-dev, linux-pci, linux-kernel, stable

Hi,

On 9/18/2026 10:24 AM, Priyank Rathod wrote:
> 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.
> 
> aer_recover_queue() returns void, so ghes_handle_aer() cannot release that
> buffer itself; ownership is handed to the AER code, which until now freed
> it only on the fully successful path. If the record cannot be enqueued, or
> if a dequeued record cannot be mapped to a pci_dev, the allocation is
> silently leaked. Under a sustained error storm this exhausts
> ghes_estatus_pool, which then breaks GHES hardware error reporting
> system-wide.
> 
> This series fixes both leak paths:
> 
> Patch 1: aer_recover_queue() when kfifo_in_spinlocked() fails because
>          aer_recover_ring (capacity 16) is full. The rejected entry is
>          freed immediately via ghes_estatus_pool_region_free().
> 
> Patch 2: aer_recover_work_func() when a dequeued entry cannot be mapped to
>          an active PCI device (pdev is NULL). The loop is restructured so
>          ghes_estatus_pool_region_free() runs unconditionally for every
>          dequeued item.
> 
> Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
> ---

I'm fine with the current approach. aer_recover_work_func() already
owns and frees the buffer, so freeing it on the enqueue failure path
keeps the ownership in one place, and it keeps the stable backport
minimal. It might be worth adding a kernel-doc comment on
aer_recover_queue() stating that it takes ownership of @aer_regs,
which must be allocated from ghes_estatus_pool, so future callers
don't trip over it.

For the series:

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

> Changes in v4:
> - Rebased onto v7.3-rc3+ (f259f446f519); applies cleanly to pci/next as
>   well. No conflicts with the Advisory Non-Fatal Error support that landed
>   in the meantime.
> - Added missing Fixes: e2abc47a5a1a ("ACPI: APEI: Fix AER info corruption
>   when error status data has multiple sections") and Cc: stable to both
>   patches; that commit (v6.7-rc1) introduced the ghes_estatus_pool
>   allocation whose ownership these paths drop.
> - Patch 1: use braces on both arms of the if/else and fix the continuation
>   alignment (checkpatch --strict).
> - Both patches now build warning-free with W=1 and CONFIG_ACPI_APEI_PCIEAER=y
>   (earlier revisions were only build-tested with APEI disabled, which
>   compiles neither of the modified functions).
> - Explained in both commit messages why the caller cannot free the buffer,
>   and when the missing-pci_dev path is reachable.
> - Cc: Lukas Wunner, who has been active in this code.
> - Link to v3: https://lore.kernel.org/r/20260803-b4-fix-aer-memleaks-v3-1-e87159611933@google.com
> 
> Changes in v3:
> - Resent to fix threading of the series.
> - Link to v2: https://lore.kernel.org/r/20260803-b4-fix-aer-memleaks-v2-1-fd199b0171fd@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 | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> ---
> base-commit: f259f446f5198d98e13756d2cd531812a0ad3064
> change-id: 20260803-b4-fix-aer-memleaks-524a1bd5e888
> 
> Best regards,

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-25 18:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-18 17:24 [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling Priyank Rathod
2026-09-18 17:24 ` [PATCH v4 1/2] PCI/AER: Fix memory leak in aer_recover_queue() on kfifo buffer overflow Priyank Rathod
2026-09-18 17:25 ` [PATCH v4 2/2] PCI/AER: Fix memory leak in aer_recover_work_func() when pci_dev is missing Priyank Rathod
2026-09-18 18:15 ` [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling Priyank Rathod
2026-09-25 18:24 ` Kuppuswamy Sathyanarayanan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®