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

Priyank Rathod posted 2 patches 6 days, 5 hours ago
drivers/pci/pcie/aer.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
[PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling
Posted by Priyank Rathod 6 days, 5 hours ago
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>
Re: [PATCH v4 0/2] PCI/AER: Fix ghes_estatus_pool memory leaks in error handling
Posted by Priyank Rathod 6 days, 4 hours ago
[+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