[PATCH v4 00/13] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko

[PATCH v4 00/13] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko

[Dave Jiang]

Fixes for pre-existing issues sashiko-bot found while reviewing patches in the
CPER, extlog and GHES paths. v1 through v3 fixed successive batches as the
review widened; see the links below.

v4 answers Tony Luck's question on v3: is there a common early point where an
oversized gdata->error_data_length gets rejected, rather than each caller
checking only "len < sizeof(*foo)"?

cper_estatus_check() walks every section and runs before any handler
on all three paths - the IRQ path via ghes_read_estatus(), the NMI path via
ghes_in_nmi_queue_one_entry(), and bert_print_all() for BERT. Patch 1 bounds
error_data_length there, so the handlers' own checks are lower bounds only, by
design, and ghes_do_proc() and ghes_handle_aer() need nothing extra.

Patch 1 also stops that bound depending on the signed helpers the question
points at. The wrap goes the wrong way to be caught: against the 72-byte v300
header, an error_data_length of 0xffffffb9 makes acpi_hest_get_record_size()
return 1 rather than something huge, so it passes "record_size > data_len" and
acpi_hest_get_next() advances by that 1. Summing in u64 and rejecting what an
int cannot carry removes the dependency.

A record needs a second, independent bound, since cper_estatus_check() sees
the record alone and never the buffer it was read into. That one belongs to
each caller: bert_print_all() bounds against the remaining BERT region, and
extlog_print() gains an ELOG_ENTRY_LEN bound in patch 3.

GHES is the one caller without it - __ghes_read_estatus() copies buf_len bytes
over the header buf_len came from, and never re-checks. No patch here:
reaching it needs something to rewrite the block while block_status is still
set, which the ACPI read-acknowledge handshake forbids, and I have no instance
of firmware doing that. Better as a separate look than a Fixes: tag and a trip
to stable for a window nobody can show is real.

The series is grouped in three parts, plus a cleanup.

Bound the record before anything walks it:

1/13:  Reject CPER records with an out-of-range error_data_length.
2/13:  Reject an error status block length that wraps a u32.
3/13:  Validate the extlog record length before walking sections.

Fix the extlog error paths, then enable them:

4/13:  Defer CXL protocol error handling to avoid a lock inversion.
5/13:  Avoid populating software AER metadata from the raw hardware buffer.
6/13:  Validate the PCIe error section length before payload access.
7/13:  Fix the CONFIG_ACPI_APEI_PCIEAER guard typo in extlog.c.

Bound each section payload before its consumers read it:

8/13:  Bound the CXL event record copy to the firmware section length.
9/13:  Validate the CXL protocol error section length before the RAS cap copy.
10/13: Read only validated fields in cper_mem_err_pack().
11/13: Validate the memory error section length before payload access.
12/13: Bound the AER info copy and sanitize software metadata in ghes.c.

Then drop an export patch 4 made redundant:

13/13: Make cxl_cper_handle_prot_err() static.

Patches 1, 2 and 10 touch drivers/firmware/efi/cper.c, closing the holes at
the shared choke point the rest of the series relies on. Patch 2 also fixes an
infinite loop in bert_print_all(), unrelated to this series but the same root
cause.

Known gaps, left for separate patches:

  - cxl_cper_print_prot_err() in drivers/firmware/efi/cper_cxl.c uses
    dvsec_len without bounding it against the section length.
  - cxl_rch_get_aer_info() in drivers/cxl/core/ras_rch.c reads the RCH AER
    capability from MMIO without clearing header_len/flit, the same class as
    patches 5 and 12.
  - struct pcie_tlp_log grew to 60 bytes for Flit mode, so the 96-byte CPER
    AER info no longer maps 1:1 onto struct aer_capability_regs: Root Error
    Command, Root Error Status and Error Source ID land in
    header_log.prefix[0..2] and print as end-to-end TLP prefixes. Patch 7
    makes this visible in extlog by fixing the guard typo, but the misdecode
    is the same before and after; patches 5 and 12 only stop the
    out-of-bounds reads around it.

v1: https://lore.kernel.org/linux-cxl/20260709162807.1957783-1-dave.jiang@intel.com/
v2: https://lore.kernel.org/linux-cxl/20260714231835.303081-1-dave.jiang@intel.com/
v3: https://lore.kernel.org/linux-cxl/20260717161647.1493259-1-dave.jiang@intel.com/

Review tags from the v3 thread are carried over. Patches 2, 10 and 13 are new
in v4 and have none. Patches 1 and 11 were reworked, so their v3 Reviewed-by
tags are dropped. Patches 4, 5, 6, 7 and 9 changed in smaller ways and keep
theirs. Patches 3, 8 and 12 are unchanged.

Changes since v3
----------------
- Regrouped into three parts: bound the record, fix and enable the extlog
  paths, then bound each section payload. v3 interleaved them, so patch 1
  checked a section before the patch establishing that contract. No code
  changed.
- Moved the extlog lock inversion fix (patch 4) ahead of the CXL protocol
  error length validation (patch 9). The former deletes
  extlog_cxl_cper_handle_prot_err(), which the v3 order plumbed a new len
  argument through three patches before removing it. No functional change.
- Patch 1: sum in u64 and reject a size the int helpers cannot carry, so the
  choke point no longer depends on acpi_hest_get_record_size() not wrapping
  (Tony Luck). The v3 "< 0" check was dead code, and bounding the u32 alone
  would have left the sum unchecked. Reviewed-by dropped; not the same check.
- New patch 2: reject an error status block length that wraps the u32 sum in
  cper_estatus_len(). A data_length of 0xffffffec makes it read back as 0,
  slipping past the extlog bound in patch 3 and leaving bert_print_all()
  advancing by zero forever (sashiko). Its commit message no longer claims
  extlog already reached cper_estatus_check_header() (sashiko).
- Patch 4: moved the cxl_cper_post_prot_err() declaration inside the
  CONFIG_ACPI_APEI_GHES block in <acpi/ghes.h> (Shuai Xue). The GHES=n plus
  EXTLOG=m failure that prompted it is unreachable, since ACPI_EXTLOG selects
  ACPI_APEI_GHES, but the declaration belongs there anyway.
- Patch 5: dropped a pointless aer alias; the local aer_regs goes straight to
  pci_print_aer().
- Patch 6: warn instead of returning silently on a short PCIe section (Shuai
  Xue), and added the Closes: link v3 omitted.
- Patch 7: corrected the Fixes tag to e778ffefa34d, the commit that added the
  "#ifdef ACPI_APEI_PCIEAER" guard, and dropped its Reported-by; sashiko-bot
  reviewed that patch rather than reporting the typo.
- Patch 9: made the two prot-err length messages distinguishable; both printed
  the same text. The second now reports dvsec_len (Shuai Xue).
- New patch 10: read only validated fields in cper_mem_err_pack(). It copied
  extended, rank, mem_array_handle and mem_dev_handle unconditionally from
  offsets 73 to 79, past the end of the 73-byte UEFI 2.1/2.2 layout (sashiko).
- Patch 11: derive the required length from the claimed validation bits. A
  single size gets it wrong both ways: 73 bytes let a 74 to 79 byte record
  through, so patch 10 could read mem_dev_handle at offset 78 off the end, and
  80 bytes rejected a 76-byte record legitimately carrying rank, or one
  claiming only bank at offset 38 (sashiko).
- New patch 13: make cxl_cper_handle_prot_err() static. Patch 4 removed its
  last external caller.
- Condensed the commit logs and comments throughout; prose only.

Changes since v2
----------------
- Dropped the standalone spin_lock_irqsave() change; a separate issue being
  handled by Terry Bowman.
- New patch 2: reject a section whose error_data_length is out of range in
  cper_estatus_check(), closing the signed-int overflow that let a crafted
  section defeat the per-caller size guards (sashiko).
- New patch 9: apply the same AER buffer sanitization to ghes_handle_aer()
  that patch 4 applies to extlog (sashiko).
- New patch 10: bound the extlog record and run cper_estatus_check() before
  walking its sections, which extlog did not do (sashiko).
- Patch 8: move the memory error length check into ghes_do_proc() so the
  report chain and arch reporter are covered too, and bound against the older
  UEFI 2.1/2.2 layout rather than the full struct (sashiko).
- Reordered so the extlog PCIe path is hardened, and CXL protocol error
  handling deferred to the workqueue, before the typo fix that activates it.

Changes since v1
----------------
- See the v2 posting for the full v1 -> v2 changelog.

Dave Jiang (13):
  efi/cper: Reject CPER records with an out-of-range error_data_length
  efi/cper: Reject an error status block length that wraps a u32
  ACPI: extlog: Validate elog record length before walking sections
  ACPI: extlog: Defer CXL protocol error handling to avoid lock
    inversion
  ACPI: extlog: Avoid populating software AER metadata from raw hardware
    buffer
  ACPI: extlog: Validate PCIe error section length before payload access
  ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
  ACPI: APEI: GHES: Bound CXL event record copy to the firmware section
    length
  ACPI: APEI: GHES: Validate CXL protocol error section length before
    RAS cap copy
  efi/cper: Read only validated fields in cper_mem_err_pack()
  ACPI: APEI: GHES: Validate memory error section length before payload
    access
  ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
  cxl/ras: Make cxl_cper_handle_prot_err() static

 drivers/acpi/acpi_extlog.c       | 49 +++++++++---------
 drivers/acpi/apei/ghes.c         | 89 +++++++++++++++++++++++++++-----
 drivers/acpi/apei/ghes_helpers.c | 18 ++++++-
 drivers/cxl/core/ras.c           |  3 +-
 drivers/firmware/efi/cper.c      | 53 ++++++++++++++++---
 include/acpi/ghes.h              |  4 ++
 include/cxl/event.h              |  6 +--
 7 files changed, 170 insertions(+), 52 deletions(-)


base-commit: 075b74841bd0065a3bda3440873c747938e69b68
-- 
2.54.0

[PATCH v4 01/13] efi/cper: Reject CPER records with an out-of-range error_data_length

[Dave Jiang]

cper_estatus_check() sizes each section with acpi_hest_get_record_size(),
which adds the firmware-controlled u32 error_data_length to the header size
as a signed int (see <acpi/ghes.h>). A value in the top sizeof(*gdata)
bytes of the u32 range wraps the sum small rather than large, so it slips
past the "record_size > data_len" check: against the 72-byte v300 header,
0xffffffb9 sizes the record at 1 and acpi_hest_get_next() walks it a byte
at a time, off the end. 0xffffffb8 sizes it at 0 and loops forever.

Sum in u64 so the check sees the real size, and reject a size the int
helpers cannot carry, since the walk advances by their return value.

This is the per-section upper bound the later "len < sizeof(*foo)" guards
rely on; they are lower bounds only.

Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=1
Fixes: 45b14a4ffcc1 ("efi: cper: Fix possible out-of-bounds access")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Do the arithmetic in u64 at the choke point instead of bounding the u32
  error_data_length against data_len, so the check no longer depends on the
  signed helpers in <acpi/ghes.h> behaving (Tony Luck). Bounding the u32
  still left a window when data_length itself was within a header of
  U32_MAX, and it did not stop acpi_hest_get_next() advancing by a wrapped
  int. The v3 "< 0" arm was dead either way, since data_len is unsigned and
  promoted the int back (Tony Luck, Shuai Xue).
- Dropped Alison's and Shuai's Reviewed-by; the check was reworked after
  they reviewed it.
---
 drivers/firmware/efi/cper.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index 06b4fdb59917..ec092729cacc 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -752,7 +752,7 @@ EXPORT_SYMBOL_GPL(cper_estatus_check_header);
 int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
 {
 	struct acpi_hest_generic_data *gdata;
-	unsigned int data_len, record_size;
+	unsigned int data_len;
 	int rc;
 
 	rc = cper_estatus_check_header(estatus);
@@ -762,11 +762,21 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
 	data_len = estatus->data_length;
 
 	apei_estatus_for_each_section(estatus, gdata) {
+		u64 record_size;
+
 		if (acpi_hest_get_size(gdata) > data_len)
 			return -EINVAL;
 
-		record_size = acpi_hest_get_record_size(gdata);
-		if (record_size > data_len)
+		/*
+		 * acpi_hest_get_record_size() sums these as a signed int (see
+		 * <acpi/ghes.h>), which wraps small for a huge
+		 * error_data_length and slips past the check below. Sum in u64,
+		 * and reject what those helpers cannot carry, since the walk
+		 * advances by their return value.
+		 */
+		record_size = (u64)acpi_hest_get_size(gdata) +
+			      gdata->error_data_length;
+		if (record_size > data_len || record_size > INT_MAX)
 			return -EINVAL;
 
 		data_len -= record_size;
-- 
2.54.0

[PATCH v4 02/13] efi/cper: Reject an error status block length that wraps a u32

[Dave Jiang]

cper_estatus_len() sums the firmware-controlled data_length (or
raw_data_offset plus raw_data_length) into a u32. A data_length of
0xffffffec wraps that sum to 0, and a length that reads back short defeats
every bound built on it: bert_print_all() passes its "remain <
estatus_len" check, then advances "estatus += estatus_len" by zero and
loops forever. GHES survives only because __ghes_check_estatus() rejects a
length below sizeof(*estatus) first.

Reject a length that cannot be expressed in a u32 in
cper_estatus_check_header(), which both of today's callers reach: GHES via
__ghes_check_estatus() and BERT via cper_estatus_check(). extlog reaches
neither yet; a later patch routes it through cper_estatus_check(), whose
ELOG_ENTRY_LEN bound needs this to hold.

Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=1
Fixes: 06d65deade9a ("ACPI, APEI, UEFI Common Platform Error Record (CPER) header")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- New patch. sashiko-bot's review of v4 pointed out that the u32 sum in
  cper_estatus_len() wraps, which bypasses the bounds added by the
  surrounding patches.
---
 drivers/firmware/efi/cper.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index ec092729cacc..ea1c999089bc 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -745,6 +745,17 @@ int cper_estatus_check_header(const struct acpi_hest_generic_status *estatus)
 	    estatus->raw_data_offset < sizeof(*estatus) + estatus->data_length)
 		return -EINVAL;
 
+	/*
+	 * cper_estatus_len() sums these into a u32, and a wrapped sum reads
+	 * back smaller than the record. Reject a length that cannot be
+	 * expressed so no caller is handed the short value.
+	 */
+	if ((u64)sizeof(*estatus) + estatus->data_length > U32_MAX)
+		return -EINVAL;
+	if (estatus->raw_data_length &&
+	    (u64)estatus->raw_data_offset + estatus->raw_data_length > U32_MAX)
+		return -EINVAL;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(cper_estatus_check_header);
-- 
2.54.0

[PATCH v4 03/13] ACPI: extlog: Validate elog record length before walking sections

[Dave Jiang]

extlog_print() copies a fixed ELOG_ENTRY_LEN (4096) bytes from the elog
record into elog_buf, then walks the sections using the firmware-controlled
data_length. Nothing keeps data_length inside the buffer, so a malformed
record walks the section pointer past elog_buf and reads adjacent memory.
Unlike the GHES paths, extlog never calls cper_estatus_check().

Reject a record longer than ELOG_ENTRY_LEN and run cper_estatus_check()
before walking the sections. The length test alone is not enough: a wrapped
length reads back short and passes it, which cper_estatus_check() catches
via the header check added earlier. Drop a malformed record with
NOTIFY_DONE and without MCE_HANDLED_EXTLOG, since extlog did not consume
it.

Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=6
Fixes: f6ec01da40e4 ("ACPI: extlog: Handle multiple records")
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/acpi_extlog.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 7ad3b36013cc..9ad0052aa20c 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -208,6 +208,10 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
 
 	tmp = (struct acpi_hest_generic_status *)elog_buf;
 
+	/* Keep the firmware-controlled data_length inside elog_buf. */
+	if (cper_estatus_len(tmp) > ELOG_ENTRY_LEN || cper_estatus_check(tmp))
+		return NOTIFY_DONE;
+
 	if (!ras_userspace_consumers()) {
 		print_extlog_rcd(NULL, tmp, cpu);
 		goto out;
-- 
2.54.0

[PATCH v4 04/13] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion

[Dave Jiang]

extlog_print() calls cxl_cper_handle_prot_err() synchronously while the
MCE notifier chain rwsem is held, and that path takes the PCI device_lock
via guard(device)(). The probe path takes the two in the opposite order,
holding device_lock while mce_register_decode_chain() takes the rwsem, so
they can deadlock AB-BA.

ghes.c already avoids this by posting protocol errors to a kfifo and
handling them from a workqueue via cxl_cper_post_prot_err(). Export that
function and call it instead.

Declare it with the other CONFIG_ACPI_APEI_GHES exports rather than at the
end of the header. No #else stub: ACPI_EXTLOG selects ACPI_APEI_GHES, so
the only caller cannot exist without it.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Moved the cxl_cper_post_prot_err() declaration inside the
  CONFIG_ACPI_APEI_GHES block (Shuai Xue).
---
 drivers/acpi/acpi_extlog.c | 21 ++-------------------
 drivers/acpi/apei/ghes.c   |  5 +++--
 include/acpi/ghes.h        |  4 ++++
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 9ad0052aa20c..f6e3da4e13e7 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -163,23 +163,6 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
 #endif
 }
 
-static void
-extlog_cxl_cper_handle_prot_err(struct cxl_cper_sec_prot_err *prot_err,
-				int severity)
-{
-#ifdef ACPI_APEI_PCIEAER
-	struct cxl_cper_prot_err_work_data wd;
-
-	if (cxl_cper_sec_prot_err_valid(prot_err))
-		return;
-
-	if (cxl_cper_setup_prot_err_work_data(&wd, prot_err, severity))
-		return;
-
-	cxl_cper_handle_prot_err(&wd);
-#endif
-}
-
 static int extlog_print(struct notifier_block *nb, unsigned long val,
 			void *data)
 {
@@ -239,8 +222,8 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
 			struct cxl_cper_sec_prot_err *prot_err =
 				acpi_hest_get_payload(gdata);
 
-			extlog_cxl_cper_handle_prot_err(prot_err,
-							gdata->error_severity);
+			cxl_cper_post_prot_err(prot_err,
+					       gdata->error_severity);
 		} else if (guid_equal(sec_type, &CPER_SEC_PCIE)) {
 			struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata);
 
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 3236a3ce79d6..a382aabf9835 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -752,8 +752,8 @@ static DEFINE_KFIFO(cxl_cper_prot_err_fifo, struct cxl_cper_prot_err_work_data,
 static DEFINE_SPINLOCK(cxl_cper_prot_err_work_lock);
 struct work_struct *cxl_cper_prot_err_work;
 
-static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
-				   int severity)
+void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
+			    int severity)
 {
 #ifdef CONFIG_ACPI_APEI_PCIEAER
 	struct cxl_cper_prot_err_work_data wd;
@@ -777,6 +777,7 @@ static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
 	schedule_work(cxl_cper_prot_err_work);
 #endif
 }
+EXPORT_SYMBOL_FOR_MODULES(cxl_cper_post_prot_err, "acpi_extlog");
 
 int cxl_cper_register_prot_err_work(struct work_struct *work)
 {
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 8d7e5caef3f1..be496bc0386f 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -85,6 +85,10 @@ int devm_ghes_register_vendor_record_notifier(struct device *dev,
 struct list_head *ghes_get_devices(void);
 
 void ghes_estatus_pool_region_free(unsigned long addr, u32 size);
+
+struct cxl_cper_sec_prot_err;
+void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
+			    int severity);
 #else
 static inline struct list_head *ghes_get_devices(void) { return NULL; }
 
-- 
2.54.0

[PATCH v4 05/13] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

[Dave Jiang]

extlog_print_pcie() casts pcie_err->aer_info straight to struct
aer_capability_regs *. That struct embeds struct pcie_tlp_log, whose
software-only header_len and flit fields sit at offset 84 - inside the
96-byte aer_info buffer - so the cast fills them with raw firmware bytes.
pcie_print_tlp_log() uses both to bound a loop over dw[], and a large
header_len walks past the end of the array.

Copy aer_info into a zeroed local struct aer_capability_regs and clear
header_len and flit before passing it on.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Dropped the now-pointless aer pointer alias.
---
 drivers/acpi/acpi_extlog.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index f6e3da4e13e7..6d5532ec0920 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -137,7 +137,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
 			      int severity)
 {
 #ifdef ACPI_APEI_PCIEAER
-	struct aer_capability_regs *aer;
+	struct aer_capability_regs aer_regs = {};
 	struct pci_dev *pdev;
 	unsigned int devfn;
 	unsigned int bus;
@@ -149,7 +149,11 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
 		return;
 
 	aer_severity = cper_severity_to_aer(severity);
-	aer = (struct aer_capability_regs *)pcie_err->aer_info;
+
+	memcpy(&aer_regs, pcie_err->aer_info, sizeof(pcie_err->aer_info));
+	aer_regs.header_log.header_len = 0;
+	aer_regs.header_log.flit = false;
+
 	domain = pcie_err->device_id.segment;
 	bus = pcie_err->device_id.bus;
 	devfn = PCI_DEVFN(pcie_err->device_id.device,
@@ -158,7 +162,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
 	if (!pdev)
 		return;
 
-	pci_print_aer(pdev, aer_severity, aer);
+	pci_print_aer(pdev, aer_severity, &aer_regs);
 	pci_dev_put(pdev);
 #endif
 }
-- 
2.54.0

[PATCH v4 06/13] ACPI: extlog: Validate PCIe error section length before payload access

[Dave Jiang]

extlog_print_pcie() reads pcie_err->validation_bits and device_id and
copies the 96-byte aer_info buffer without checking that
gdata->error_data_length is big enough for a struct cper_sec_pcie. The
cper_estatus_check() call added earlier keeps the read inside the estatus
block, but a short section still gets stale adjacent bytes treated as PCIe
error data.

Reject a section too small to hold the record before touching any field,
and warn: a truncated section means firmware is emitting malformed records.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Warn instead of returning silently, matching the other length
  rejections in the series (Shuai Xue).
- Added the Closes: link to the sashiko report, which v3 omitted.
---
 drivers/acpi/acpi_extlog.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 6d5532ec0920..3aec73187b51 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -134,7 +134,7 @@ static int print_extlog_rcd(const char *pfx,
 }
 
 static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
-			      int severity)
+			      int severity, u32 len)
 {
 #ifdef ACPI_APEI_PCIEAER
 	struct aer_capability_regs aer_regs = {};
@@ -144,6 +144,12 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
 	int aer_severity;
 	int domain;
 
+	if (len < sizeof(*pcie_err)) {
+		pr_warn_ratelimited(FW_WARN
+				    "PCIe error section too small (%u)\n", len);
+		return;
+	}
+
 	if (!(pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
 	      pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO))
 		return;
@@ -231,7 +237,8 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
 		} else if (guid_equal(sec_type, &CPER_SEC_PCIE)) {
 			struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata);
 
-			extlog_print_pcie(pcie_err, gdata->error_severity);
+			extlog_print_pcie(pcie_err, gdata->error_severity,
+					  gdata->error_data_length);
 		} else {
 			void *err = acpi_hest_get_payload(gdata);
 
-- 
2.54.0

[PATCH v4 07/13] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

[Dave Jiang]

The guard reads "#ifdef ACPI_APEI_PCIEAER" rather than
"#ifdef CONFIG_ACPI_APEI_PCIEAER". That symbol is never defined, so the
extlog PCIe AER handling is always compiled out.

Use the CONFIG_ prefixed symbol.

Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Corrected the Fixes tag: e778ffefa34d added the "#ifdef ACPI_APEI_PCIEAER"
  guard, not 95350effc3ad.
- Dropped Reported-by: sashiko-bot; it reviewed this patch rather than
  reporting the typo.
---
 drivers/acpi/acpi_extlog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 3aec73187b51..ebedf3b136a8 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -136,7 +136,7 @@ static int print_extlog_rcd(const char *pfx,
 static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
 			      int severity, u32 len)
 {
-#ifdef ACPI_APEI_PCIEAER
+#ifdef CONFIG_ACPI_APEI_PCIEAER
 	struct aer_capability_regs aer_regs = {};
 	struct pci_dev *pdev;
 	unsigned int devfn;
-- 
2.54.0

[PATCH v4 08/13] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

[Dave Jiang]

cxl_cper_post_event() copies a fixed sizeof(struct cxl_cper_event_rec)
out of the firmware CPER section without checking how long the section
actually is, so a short one reads past the record.

Pass gdata->error_data_length in and reject a section too small to hold
the record before the copy.

Reported-by: sashiko-bot@kernel.org
Link: https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=6
Fixes: 5e4a264bf8b5 ("acpi/ghes: Process CXL Component Events")
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/apei/ghes.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index a382aabf9835..e5f8dbd17017 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -816,10 +816,15 @@ static DEFINE_SPINLOCK(cxl_cper_work_lock);
 struct work_struct *cxl_cper_work;
 
 static void cxl_cper_post_event(enum cxl_event_type event_type,
-				struct cxl_cper_event_rec *rec)
+				struct cxl_cper_event_rec *rec, u32 len)
 {
 	struct cxl_cper_work_data wd;
 
+	if (len < sizeof(*rec)) {
+		pr_err(FW_WARN "CXL CPER section too small (%u)\n", len);
+		return;
+	}
+
 	if (rec->hdr.length <= sizeof(rec->hdr) ||
 	    rec->hdr.length > sizeof(*rec)) {
 		pr_err(FW_WARN "CXL CPER Invalid section length (%u)\n",
@@ -950,15 +955,18 @@ static void ghes_do_proc(struct ghes *ghes,
 		} else if (guid_equal(sec_type, &CPER_SEC_CXL_GEN_MEDIA_GUID)) {
 			struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
 
-			cxl_cper_post_event(CXL_CPER_EVENT_GEN_MEDIA, rec);
+			cxl_cper_post_event(CXL_CPER_EVENT_GEN_MEDIA, rec,
+					    gdata->error_data_length);
 		} else if (guid_equal(sec_type, &CPER_SEC_CXL_DRAM_GUID)) {
 			struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
 
-			cxl_cper_post_event(CXL_CPER_EVENT_DRAM, rec);
+			cxl_cper_post_event(CXL_CPER_EVENT_DRAM, rec,
+					    gdata->error_data_length);
 		} else if (guid_equal(sec_type, &CPER_SEC_CXL_MEM_MODULE_GUID)) {
 			struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
 
-			cxl_cper_post_event(CXL_CPER_EVENT_MEM_MODULE, rec);
+			cxl_cper_post_event(CXL_CPER_EVENT_MEM_MODULE, rec,
+					    gdata->error_data_length);
 		} else {
 			void *err = acpi_hest_get_payload(gdata);
 
-- 
2.54.0

[PATCH v4 09/13] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy

[Dave Jiang]

cxl_cper_setup_prot_err_work_data() locates the RAS Capability block at
prot_err + sizeof(*prot_err) + dvsec_len and copies it, but dvsec_len is
firmware controlled and never validated, so it can point the copy outside
the section.

Extend cxl_cper_sec_prot_err_valid() to check that the section can hold
the header, and that the header, DVSEC and RAS Capability block together
fit the reported section length.

Reported-by: sashiko-bot@kernel.org
Link: https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=6
Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
Fixes: 315c2f0b53ba ("acpi/ghes, cper: Recognize and cache CXL Protocol errors")
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Made the two prot-err length messages distinguishable; the second now
  reports dvsec_len (Shuai Xue).
- Reordered after the extlog lock inversion fix, so the new len argument
  goes only to cxl_cper_post_prot_err() rather than also to the
  now-deleted extlog_cxl_cper_handle_prot_err().
---
 drivers/acpi/acpi_extlog.c       |  3 ++-
 drivers/acpi/apei/ghes.c         |  7 ++++---
 drivers/acpi/apei/ghes_helpers.c | 18 +++++++++++++++++-
 include/acpi/ghes.h              |  2 +-
 include/cxl/event.h              |  4 ++--
 5 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index ebedf3b136a8..d32306c36511 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -233,7 +233,8 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
 				acpi_hest_get_payload(gdata);
 
 			cxl_cper_post_prot_err(prot_err,
-					       gdata->error_severity);
+					       gdata->error_severity,
+					       gdata->error_data_length);
 		} else if (guid_equal(sec_type, &CPER_SEC_PCIE)) {
 			struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata);
 
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index e5f8dbd17017..b8dbd99da47e 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -753,12 +753,12 @@ static DEFINE_SPINLOCK(cxl_cper_prot_err_work_lock);
 struct work_struct *cxl_cper_prot_err_work;
 
 void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
-			    int severity)
+			    int severity, u32 len)
 {
 #ifdef CONFIG_ACPI_APEI_PCIEAER
 	struct cxl_cper_prot_err_work_data wd;
 
-	if (cxl_cper_sec_prot_err_valid(prot_err))
+	if (cxl_cper_sec_prot_err_valid(prot_err, len))
 		return;
 
 	guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
@@ -951,7 +951,8 @@ static void ghes_do_proc(struct ghes *ghes,
 		} else if (guid_equal(sec_type, &CPER_SEC_CXL_PROT_ERR)) {
 			struct cxl_cper_sec_prot_err *prot_err = acpi_hest_get_payload(gdata);
 
-			cxl_cper_post_prot_err(prot_err, gdata->error_severity);
+			cxl_cper_post_prot_err(prot_err, gdata->error_severity,
+					       gdata->error_data_length);
 		} else if (guid_equal(sec_type, &CPER_SEC_CXL_GEN_MEDIA_GUID)) {
 			struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
 
diff --git a/drivers/acpi/apei/ghes_helpers.c b/drivers/acpi/apei/ghes_helpers.c
index bc7111b740af..df41b993f413 100644
--- a/drivers/acpi/apei/ghes_helpers.c
+++ b/drivers/acpi/apei/ghes_helpers.c
@@ -5,8 +5,15 @@
 #include <linux/aer.h>
 #include <cxl/event.h>
 
-int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err)
+int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err, u32 len)
 {
+	if (len < sizeof(*prot_err)) {
+		pr_err_ratelimited(FW_WARN
+				   "CXL CPER prot err section too small (%u)\n",
+				   len);
+		return -EINVAL;
+	}
+
 	if (!(prot_err->valid_bits & PROT_ERR_VALID_AGENT_ADDRESS)) {
 		pr_err_ratelimited("CXL CPER invalid agent type\n");
 		return -EINVAL;
@@ -23,6 +30,15 @@ int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err)
 		return -EINVAL;
 	}
 
+	/* The RAS Capability block sits after a firmware-sized DVSEC. */
+	if (sizeof(*prot_err) + prot_err->dvsec_len +
+	    sizeof(struct cxl_ras_capability_regs) > len) {
+		pr_err_ratelimited(FW_WARN
+				   "CXL CPER prot err DVSEC (%u) overruns section (%u)\n",
+				   prot_err->dvsec_len, len);
+		return -EINVAL;
+	}
+
 	if ((prot_err->agent_type == RCD || prot_err->agent_type == DEVICE ||
 	     prot_err->agent_type == LD || prot_err->agent_type == FMLD) &&
 	    !(prot_err->valid_bits & PROT_ERR_VALID_SERIAL_NUMBER))
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index be496bc0386f..7acf209061ea 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -88,7 +88,7 @@ void ghes_estatus_pool_region_free(unsigned long addr, u32 size);
 
 struct cxl_cper_sec_prot_err;
 void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
-			    int severity);
+			    int severity, u32 len);
 #else
 static inline struct list_head *ghes_get_devices(void) { return NULL; }
 
diff --git a/include/cxl/event.h b/include/cxl/event.h
index ff97fea718d2..912305bee3bc 100644
--- a/include/cxl/event.h
+++ b/include/cxl/event.h
@@ -321,13 +321,13 @@ static inline int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data
 #endif
 
 #ifdef CONFIG_ACPI_APEI_PCIEAER
-int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err);
+int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err, u32 len);
 int cxl_cper_setup_prot_err_work_data(struct cxl_cper_prot_err_work_data *wd,
 				      struct cxl_cper_sec_prot_err *prot_err,
 				      int severity);
 #else
 static inline int
-cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err)
+cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err, u32 len)
 {
 	return -EOPNOTSUPP;
 }
-- 
2.54.0

[PATCH v4 10/13] efi/cper: Read only validated fields in cper_mem_err_pack()

[Dave Jiang]

cper_mem_err_pack() copies extended, rank, mem_array_handle and
mem_dev_handle unconditionally. Those live at offsets 73 to 79, past the
end of struct cper_sec_mem_err_old, the 73-byte UEFI 2.1/2.2 layout that
older firmware still emits and that cper_estatus_print_section() admits.
On such a record the copy reads up to seven bytes past the payload, and off
the end of the error status block when that section is the last one.

Copy each of the four only when its validation bit is set, and zero it
otherwise. Nothing is lost: a 2.1/2.2 record leaves those bits clear, and
every consumer of struct cper_mem_err_compact already gates the fields on
the same bits. Zeroing also stops callers reading them back out of the
uninitialised on-stack struct they pass in.

Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=7
Fixes: 2dfb7d51a61d ("trace, RAS: Add eMCA trace event interface")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- New patch. sashiko-bot pointed out that bounding the memory error
  section at the 73-byte layout still leaves cper_mem_err_pack() reading
  offsets 73 to 79.
---
 drivers/firmware/efi/cper.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index ea1c999089bc..6c64a0f06a4e 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -389,10 +389,28 @@ void cper_mem_err_pack(const struct cper_sec_mem_err *mem,
 	cmem->requestor_id = mem->requestor_id;
 	cmem->responder_id = mem->responder_id;
 	cmem->target_id = mem->target_id;
-	cmem->extended = mem->extended;
-	cmem->rank = mem->rank;
-	cmem->mem_array_handle = mem->mem_array_handle;
-	cmem->mem_dev_handle = mem->mem_dev_handle;
+
+	/*
+	 * These four sit past the end of the UEFI 2.1/2.2 layout, which older
+	 * firmware still emits, so reading them unconditionally runs off a
+	 * short record. Every consumer of the compact record gates them on the
+	 * same validation bits, so leave them zero when firmware does not
+	 * claim them.
+	 */
+	cmem->extended = 0;
+	cmem->rank = 0;
+	cmem->mem_array_handle = 0;
+	cmem->mem_dev_handle = 0;
+
+	if (mem->validation_bits &
+	    (CPER_MEM_VALID_ROW_EXT | CPER_MEM_VALID_CHIP_ID))
+		cmem->extended = mem->extended;
+	if (mem->validation_bits & CPER_MEM_VALID_RANK_NUMBER)
+		cmem->rank = mem->rank;
+	if (mem->validation_bits & CPER_MEM_VALID_CARD_HANDLE)
+		cmem->mem_array_handle = mem->mem_array_handle;
+	if (mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE)
+		cmem->mem_dev_handle = mem->mem_dev_handle;
 }
 EXPORT_SYMBOL_GPL(cper_mem_err_pack);
 
-- 
2.54.0

[PATCH v4 11/13] ACPI: APEI: GHES: Validate memory error section length before payload access

[Dave Jiang]

ghes_do_proc() hands the CPER_SEC_PLATFORM_MEM payload to the report chain,
arch_apei_report_mem_error() and ghes_handle_memory_failure() without
checking gdata->error_data_length. All three read validation_bits and
physical_addr, at offsets 0 and 16, so a shorter section reads past the
record.

Check the length once in ghes_do_proc(), before any consumer runs. Take the
73-byte struct cper_sec_mem_err_old as the floor: older firmware
legitimately emits that UEFI 2.1/2.2 layout, and it makes validation_bits
safe to read.

The fields from "extended" on are absent from that layout, so also require
whatever length the validation bits claim. Derive it per field rather than
demanding the full 80 bytes: rank needs only 76, and the BANK_GROUP and
BANK_ADDRESS bits describe "bank" at offset 38, which every record carries.
cper_print_mem() tests the whole range above CPER_MEM_VALID_RANK_NUMBER
against the 73-byte size instead, which both admits 74 to 79 and rejects a
record claiming only "bank".

Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=7
Fixes: ca104edc1784 ("ACPI, APEI, GHES: Cleanup ghes memory error handling")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- Derive the required length from the claimed validation bits instead of
  testing the whole mask against one size. The blanket form rejected a
  76-byte record that legitimately carries rank, and any short record
  claiming only bank (sashiko).
- Dropped Alison's and Shuai's Reviewed-by; the check gained a helper and
  is no longer the patch they reviewed.
---
 drivers/acpi/apei/ghes.c | 41 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index b8dbd99da47e..0cc1e6383635 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -915,6 +915,28 @@ static void ghes_log_hwerr(int sev, guid_t *sec_type)
 	hwerr_log_error_type(HWERR_RECOV_OTHERS);
 }
 
+/*
+ * The fields from "extended" on are absent from the 73-byte UEFI 2.1/2.2
+ * layout that older firmware still emits. Return the length needed for the
+ * fields the validation bits claim, so over-claiming is rejected without
+ * rejecting an honest short record.
+ */
+static u32 ghes_mem_err_min_len(u64 validation_bits)
+{
+	u32 len = sizeof(struct cper_sec_mem_err_old);
+
+	if (validation_bits & (CPER_MEM_VALID_ROW_EXT | CPER_MEM_VALID_CHIP_ID))
+		len = offsetof(struct cper_sec_mem_err, rank);
+	if (validation_bits & CPER_MEM_VALID_RANK_NUMBER)
+		len = offsetof(struct cper_sec_mem_err, mem_array_handle);
+	if (validation_bits & CPER_MEM_VALID_CARD_HANDLE)
+		len = offsetof(struct cper_sec_mem_err, mem_dev_handle);
+	if (validation_bits & CPER_MEM_VALID_MODULE_HANDLE)
+		len = sizeof(struct cper_sec_mem_err);
+
+	return len;
+}
+
 static void ghes_do_proc(struct ghes *ghes,
 			 const struct acpi_hest_generic_status *estatus)
 {
@@ -940,6 +962,25 @@ static void ghes_do_proc(struct ghes *ghes,
 		if (guid_equal(sec_type, &CPER_SEC_PLATFORM_MEM)) {
 			struct cper_sec_mem_err *mem_err = acpi_hest_get_payload(gdata);
 
+			/*
+			 * Check once for all three consumers below. The 73-byte
+			 * UEFI 2.1/2.2 layout is the floor, matching
+			 * cper_estatus_print_section() and making
+			 * validation_bits safe to read.
+			 */
+			if (gdata->error_data_length <
+			    sizeof(struct cper_sec_mem_err_old))
+				continue;
+
+			/* Then require what the claimed fields actually need. */
+			if (gdata->error_data_length <
+			    ghes_mem_err_min_len(mem_err->validation_bits)) {
+				pr_warn_ratelimited(FW_WARN GHES_PFX
+						    "memory error section too small (%u) for the fields it claims\n",
+						    gdata->error_data_length);
+				continue;
+			}
+
 			atomic_notifier_call_chain(&ghes_report_chain, sev, mem_err);
 
 			arch_apei_report_mem_error(sev, mem_err);
-- 
2.54.0

[PATCH v4 12/13] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata

[Dave Jiang]

ghes_handle_aer() copies sizeof(struct aer_capability_regs) out of the
fixed 96-byte pcie_err->aer_info. The struct is larger, so the copy reads
past the section. It also fills the software-only header_len and flit
fields of the embedded struct pcie_tlp_log from raw firmware bytes, and
pcie_print_tlp_log() uses both to bound a loop over dw[], so a large value
walks past the array. Nothing checks that the section can hold a struct
cper_sec_pcie either.

Validate error_data_length, zero the destination, bound the copy to the
96-byte source, and clear header_len and flit, mirroring the
extlog_print_pcie() fix.

Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=3
Fixes: 7e077e6707b3 ("PCI/ERR: Handle TLP Log in Flit mode")
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/apei/ghes.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 0cc1e6383635..d6643127a2df 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -642,11 +642,14 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
 #ifdef CONFIG_ACPI_APEI_PCIEAER
 	struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata);
 
+	if (gdata->error_data_length < sizeof(*pcie_err))
+		return;
+
 	if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
 	    pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) {
+		struct aer_capability_regs *aer_info;
 		unsigned int devfn;
 		int aer_severity;
-		u8 *aer_info;
 
 		devfn = PCI_DEVFN(pcie_err->device_id.device,
 				  pcie_err->device_id.function);
@@ -664,13 +667,22 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
 						  sizeof(struct aer_capability_regs));
 		if (!aer_info)
 			return;
-		memcpy(aer_info, pcie_err->aer_info, sizeof(struct aer_capability_regs));
+
+		/*
+		 * The CPER source is a fixed 96 bytes, shorter than struct
+		 * aer_capability_regs, so bound the copy to it. header_len and
+		 * flit are software-only and land inside those 96 bytes; clear
+		 * them so firmware cannot drive the pcie_print_tlp_log() loop
+		 * over dw[] out of bounds.
+		 */
+		memset(aer_info, 0, sizeof(struct aer_capability_regs));
+		memcpy(aer_info, pcie_err->aer_info, sizeof(pcie_err->aer_info));
+		aer_info->header_log.header_len = 0;
+		aer_info->header_log.flit = false;
 
 		aer_recover_queue(pcie_err->device_id.segment,
 				  pcie_err->device_id.bus,
-				  devfn, aer_severity,
-				  (struct aer_capability_regs *)
-				  aer_info);
+				  devfn, aer_severity, aer_info);
 	}
 #endif
 }
-- 
2.54.0

[PATCH v4 13/13] cxl/ras: Make cxl_cper_handle_prot_err() static

[Dave Jiang]

The extlog caller went away when extlog switched to
cxl_cper_post_prot_err(), leaving cxl_cper_prot_err_work_fn() in the same
file as the only caller.

Drop the export and the declaration in <cxl/event.h>, and make it static.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v4:
- New patch. The extlog lock inversion fix earlier in the series removes the
  last external caller, leaving the export and the <cxl/event.h> declaration
  dead.
---
 drivers/cxl/core/ras.c | 3 +--
 include/cxl/event.h    | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
index 99fb00949c2f..eceb33bc534b 100644
--- a/drivers/cxl/core/ras.c
+++ b/drivers/cxl/core/ras.c
@@ -77,7 +77,7 @@ static int match_memdev_by_parent(struct device *dev, const void *uport)
 	return 0;
 }
 
-void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
+static void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
 {
 	unsigned int devfn = PCI_DEVFN(data->prot_err.agent_addr.device,
 				       data->prot_err.agent_addr.function);
@@ -118,7 +118,6 @@ void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
 	else
 		cxl_cper_trace_uncorr_prot_err(cxlmd, data->ras_cap);
 }
-EXPORT_SYMBOL_GPL(cxl_cper_handle_prot_err);
 
 static void cxl_cper_prot_err_work_fn(struct work_struct *work)
 {
diff --git a/include/cxl/event.h b/include/cxl/event.h
index 912305bee3bc..894eccbe5cbe 100644
--- a/include/cxl/event.h
+++ b/include/cxl/event.h
@@ -340,6 +340,4 @@ cxl_cper_setup_prot_err_work_data(struct cxl_cper_prot_err_work_data *wd,
 }
 #endif
 
-void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *wd);
-
 #endif /* _LINUX_CXL_EVENT_H */
-- 
2.54.0

Re: [PATCH v4 01/13] efi/cper: Reject CPER records with an out-of-range error_data_length

[Jonathan Cameron]

On Mon, 24 Aug 2026 10:49:24 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> cper_estatus_check() sizes each section with acpi_hest_get_record_size(),
> which adds the firmware-controlled u32 error_data_length to the header size
> as a signed int (see <acpi/ghes.h>). A value in the top sizeof(*gdata)
> bytes of the u32 range wraps the sum small rather than large, so it slips
> past the "record_size > data_len" check: against the 72-byte v300 header,
> 0xffffffb9 sizes the record at 1 and acpi_hest_get_next() walks it a byte
> at a time, off the end. 0xffffffb8 sizes it at 0 and loops forever.
> 
> Sum in u64 so the check sees the real size, and reject a size the int
> helpers cannot carry, since the walk advances by their return value.
> 
> This is the per-section upper bound the later "len < sizeof(*foo)" guards
> rely on; they are lower bounds only.
> 
> Reported-by: sashiko-bot@kernel.org
> Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=1
> Fixes: 45b14a4ffcc1 ("efi: cper: Fix possible out-of-bounds access")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
I know I'm late to the discussion but maybe it would just be simpler to
use check_add_overflow()?

> ---
> v4:
> - Do the arithmetic in u64 at the choke point instead of bounding the u32
>   error_data_length against data_len, so the check no longer depends on the
>   signed helpers in <acpi/ghes.h> behaving (Tony Luck). Bounding the u32
>   still left a window when data_length itself was within a header of
>   U32_MAX, and it did not stop acpi_hest_get_next() advancing by a wrapped
>   int. The v3 "< 0" arm was dead either way, since data_len is unsigned and
>   promoted the int back (Tony Luck, Shuai Xue).
> - Dropped Alison's and Shuai's Reviewed-by; the check was reworked after
>   they reviewed it.
> ---
>  drivers/firmware/efi/cper.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index 06b4fdb59917..ec092729cacc 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -752,7 +752,7 @@ EXPORT_SYMBOL_GPL(cper_estatus_check_header);
>  int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
>  {
>  	struct acpi_hest_generic_data *gdata;
> -	unsigned int data_len, record_size;
> +	unsigned int data_len;
>  	int rc;
>  
>  	rc = cper_estatus_check_header(estatus);
> @@ -762,11 +762,21 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
>  	data_len = estatus->data_length;
>  
>  	apei_estatus_for_each_section(estatus, gdata) {
> +		u64 record_size;
> +
>  		if (acpi_hest_get_size(gdata) > data_len)
>  			return -EINVAL;
>  
> -		record_size = acpi_hest_get_record_size(gdata);
> -		if (record_size > data_len)
> +		/*
> +		 * acpi_hest_get_record_size() sums these as a signed int (see
> +		 * <acpi/ghes.h>), which wraps small for a huge
> +		 * error_data_length and slips past the check below. Sum in u64,
> +		 * and reject what those helpers cannot carry, since the walk
> +		 * advances by their return value.
> +		 */
> +		record_size = (u64)acpi_hest_get_size(gdata) +
> +			      gdata->error_data_length;

I'm late to the game obviously and what you have works but could this have
used some explicit overflow checking?  Something like

		if (check_add_overflow(acpi_hest_get_size(gdata),
				       gdata->error_data_length, &record_size))
			return -EINVAL;

		if (record_size > data_len)
			return -EINVAL;

That uses the compiler __builtin_add_overflow() which checks if the infinite
precision result of the sum of the parameters would have wrapped when written
to the output one.

I think you could then drop the earlier check as well as
record_size is at least as big as acpi_hest_get_size() and we know there
was no wrap around.

	
> +		if (record_size > data_len || record_size > INT_MAX)
>  			return -EINVAL;
>  
>  		data_len -= record_size;

Re: [PATCH v4 02/13] efi/cper: Reject an error status block length that wraps a u32

[Jonathan Cameron]

On Mon, 24 Aug 2026 10:49:25 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> cper_estatus_len() sums the firmware-controlled data_length (or
> raw_data_offset plus raw_data_length) into a u32. A data_length of
> 0xffffffec wraps that sum to 0, and a length that reads back short defeats
> every bound built on it: bert_print_all() passes its "remain <
> estatus_len" check, then advances "estatus += estatus_len" by zero and
> loops forever. GHES survives only because __ghes_check_estatus() rejects a
> length below sizeof(*estatus) first.
> 
> Reject a length that cannot be expressed in a u32 in
> cper_estatus_check_header(), which both of today's callers reach: GHES via
> __ghes_check_estatus() and BERT via cper_estatus_check(). extlog reaches
> neither yet; a later patch routes it through cper_estatus_check(), whose
> ELOG_ENTRY_LEN bound needs this to hold.
> 
> Reported-by: sashiko-bot@kernel.org
> Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=1
> Fixes: 06d65deade9a ("ACPI, APEI, UEFI Common Platform Error Record (CPER) header")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Seems fine to me and to use the overflow.h stuff here we'd have to invent some
local variables which rather outweighs their documentation benefit.

Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

> ---
> v4:
> - New patch. sashiko-bot's review of v4 pointed out that the u32 sum in
>   cper_estatus_len() wraps, which bypasses the bounds added by the
>   surrounding patches.
> ---
>  drivers/firmware/efi/cper.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index ec092729cacc..ea1c999089bc 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -745,6 +745,17 @@ int cper_estatus_check_header(const struct acpi_hest_generic_status *estatus)
>  	    estatus->raw_data_offset < sizeof(*estatus) + estatus->data_length)
>  		return -EINVAL;
>  
> +	/*
> +	 * cper_estatus_len() sums these into a u32, and a wrapped sum reads
> +	 * back smaller than the record. Reject a length that cannot be
> +	 * expressed so no caller is handed the short value.
> +	 */
> +	if ((u64)sizeof(*estatus) + estatus->data_length > U32_MAX)
> +		return -EINVAL;
> +	if (estatus->raw_data_length &&
> +	    (u64)estatus->raw_data_offset + estatus->raw_data_length > U32_MAX)
> +		return -EINVAL;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(cper_estatus_check_header);

Re: [PATCH v4 03/13] ACPI: extlog: Validate elog record length before walking sections

[Jonathan Cameron]

On Mon, 24 Aug 2026 10:49:26 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> extlog_print() copies a fixed ELOG_ENTRY_LEN (4096) bytes from the elog
> record into elog_buf, then walks the sections using the firmware-controlled
> data_length. Nothing keeps data_length inside the buffer, so a malformed
> record walks the section pointer past elog_buf and reads adjacent memory.
> Unlike the GHES paths, extlog never calls cper_estatus_check().
> 
> Reject a record longer than ELOG_ENTRY_LEN and run cper_estatus_check()
> before walking the sections. The length test alone is not enough: a wrapped
> length reads back short and passes it, which cper_estatus_check() catches
> via the header check added earlier. Drop a malformed record with
> NOTIFY_DONE and without MCE_HANDLED_EXTLOG, since extlog did not consume
> it.
> 
> Reported-by: sashiko-bot@kernel.org
> Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=6
> Fixes: f6ec01da40e4 ("ACPI: extlog: Handle multiple records")
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/acpi/acpi_extlog.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 7ad3b36013cc..9ad0052aa20c 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -208,6 +208,10 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
>  
>  	tmp = (struct acpi_hest_generic_status *)elog_buf;
>  
> +	/* Keep the firmware-controlled data_length inside elog_buf. */
> +	if (cper_estatus_len(tmp) > ELOG_ENTRY_LEN || cper_estatus_check(tmp))

Why this order?  To me checking if we are in crazy world (overflow) before
doing anything with the overflowed value makes more sense. So that would be swapping
the two conditions.

> +		return NOTIFY_DONE;
> +
>  	if (!ras_userspace_consumers()) {
>  		print_extlog_rcd(NULL, tmp, cpu);
>  		goto out;

Re: [PATCH v4 04/13] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion

[Jonathan Cameron]

On Mon, 24 Aug 2026 10:49:27 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> extlog_print() calls cxl_cper_handle_prot_err() synchronously while the
> MCE notifier chain rwsem is held, and that path takes the PCI device_lock
> via guard(device)(). The probe path takes the two in the opposite order,
> holding device_lock while mce_register_decode_chain() takes the rwsem, so
> they can deadlock AB-BA.
> 
> ghes.c already avoids this by posting protocol errors to a kfifo and
> handling them from a workqueue via cxl_cper_post_prot_err(). Export that
> function and call it instead.
> 
> Declare it with the other CONFIG_ACPI_APEI_GHES exports rather than at the
> end of the header. No #else stub: ACPI_EXTLOG selects ACPI_APEI_GHES, so
> the only caller cannot exist without it.
> 
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Makes sense to standardize irrespective of all the other reasons!
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Re: [PATCH v4 05/13] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

[Jonathan Cameron]

On Mon, 24 Aug 2026 10:49:28 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> extlog_print_pcie() casts pcie_err->aer_info straight to struct
> aer_capability_regs *. That struct embeds struct pcie_tlp_log, whose
> software-only header_len and flit fields sit at offset 84 - inside the
> 96-byte aer_info buffer - so the cast fills them with raw firmware bytes.
> pcie_print_tlp_log() uses both to bound a loop over dw[], and a large
> header_len walks past the end of the array.
> 
> Copy aer_info into a zeroed local struct aer_capability_regs and clear
> header_len and flit before passing it on.

The existing code is very odd. It might be nice to clean it up more
generally so that we can handle the rest of the aer_info buffer
via a definition that matches the hardware spec. 

For now I'd be tempted to just copy the bit of the structure that
is matching the spec - side effect being the rest ends up as zero
including the two things you clear.

So just copy to first 4 dw of the header log. 

Jonathan



> 
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
> Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v4:
> - Dropped the now-pointless aer pointer alias.
> ---
>  drivers/acpi/acpi_extlog.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index f6e3da4e13e7..6d5532ec0920 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -137,7 +137,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>  			      int severity)
>  {
>  #ifdef ACPI_APEI_PCIEAER
> -	struct aer_capability_regs *aer;
> +	struct aer_capability_regs aer_regs = {};
>  	struct pci_dev *pdev;
>  	unsigned int devfn;
>  	unsigned int bus;
> @@ -149,7 +149,11 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>  		return;
>  
>  	aer_severity = cper_severity_to_aer(severity);
> -	aer = (struct aer_capability_regs *)pcie_err->aer_info;
> +
> +	memcpy(&aer_regs, pcie_err->aer_info, sizeof(pcie_err->aer_info));
> +	aer_regs.header_log.header_len = 0;
> +	aer_regs.header_log.flit = false;
> +
>  	domain = pcie_err->device_id.segment;
>  	bus = pcie_err->device_id.bus;
>  	devfn = PCI_DEVFN(pcie_err->device_id.device,
> @@ -158,7 +162,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>  	if (!pdev)
>  		return;
>  
> -	pci_print_aer(pdev, aer_severity, aer);
> +	pci_print_aer(pdev, aer_severity, &aer_regs);
>  	pci_dev_put(pdev);
>  #endif
>  }

Re: [PATCH v4 06/13] ACPI: extlog: Validate PCIe error section length before payload access

[Jonathan Cameron]

On Mon, 24 Aug 2026 10:49:29 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> extlog_print_pcie() reads pcie_err->validation_bits and device_id and
> copies the 96-byte aer_info buffer without checking that
> gdata->error_data_length is big enough for a struct cper_sec_pcie. The
> cper_estatus_check() call added earlier keeps the read inside the estatus
> block, but a short section still gets stale adjacent bytes treated as PCIe
> error data.
> 
> Reject a section too small to hold the record before touching any field,
> and warn: a truncated section means firmware is emitting malformed records.
> 
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
> Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Re: [PATCH v4 07/13] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

[Jonathan Cameron]

On Mon, 24 Aug 2026 10:49:30 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> The guard reads "#ifdef ACPI_APEI_PCIEAER" rather than
> "#ifdef CONFIG_ACPI_APEI_PCIEAER". That symbol is never defined, so the
> extlog PCIe AER handling is always compiled out.
> 
> Use the CONFIG_ prefixed symbol.
> 
> Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Yikes.

Raises some testing questions. I guess a last minute edit perhaps.

Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>


> ---
> v4:
> - Corrected the Fixes tag: e778ffefa34d added the "#ifdef ACPI_APEI_PCIEAER"
>   guard, not 95350effc3ad.
> - Dropped Reported-by: sashiko-bot; it reviewed this patch rather than
>   reporting the typo.
> ---
>  drivers/acpi/acpi_extlog.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 3aec73187b51..ebedf3b136a8 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -136,7 +136,7 @@ static int print_extlog_rcd(const char *pfx,
>  static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>  			      int severity, u32 len)
>  {
> -#ifdef ACPI_APEI_PCIEAER
> +#ifdef CONFIG_ACPI_APEI_PCIEAER
>  	struct aer_capability_regs aer_regs = {};
>  	struct pci_dev *pdev;
>  	unsigned int devfn;

Re: [PATCH v4 08/13] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

[Jonathan Cameron]

On Mon, 24 Aug 2026 10:49:31 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> cxl_cper_post_event() copies a fixed sizeof(struct cxl_cper_event_rec)
> out of the firmware CPER section without checking how long the section
> actually is, so a short one reads past the record.
> 
> Pass gdata->error_data_length in and reject a section too small to hold
> the record before the copy.
> 
> Reported-by: sashiko-bot@kernel.org
> Link: https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=6
> Fixes: 5e4a264bf8b5 ("acpi/ghes: Process CXL Component Events")
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Re: [PATCH v4 09/13] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy

[Jonathan Cameron]

On Mon, 24 Aug 2026 10:49:32 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> cxl_cper_setup_prot_err_work_data() locates the RAS Capability block at
> prot_err + sizeof(*prot_err) + dvsec_len and copies it, but dvsec_len is
> firmware controlled and never validated, so it can point the copy outside
> the section.
> 
> Extend cxl_cper_sec_prot_err_valid() to check that the section can hold
> the header, and that the header, DVSEC and RAS Capability block together
> fit the reported section length.
> 
> Reported-by: sashiko-bot@kernel.org
> Link: https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=6
> Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
> Fixes: 315c2f0b53ba ("acpi/ghes, cper: Recognize and cache CXL Protocol errors")
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

Re: [PATCH v4 01/13] efi/cper: Reject CPER records with an out-of-range error_data_length

[Dave Jiang]

On 8/24/26 2:57 PM, Jonathan Cameron wrote:
> On Mon, 24 Aug 2026 10:49:24 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> cper_estatus_check() sizes each section with acpi_hest_get_record_size(),
>> which adds the firmware-controlled u32 error_data_length to the header size
>> as a signed int (see <acpi/ghes.h>). A value in the top sizeof(*gdata)
>> bytes of the u32 range wraps the sum small rather than large, so it slips
>> past the "record_size > data_len" check: against the 72-byte v300 header,
>> 0xffffffb9 sizes the record at 1 and acpi_hest_get_next() walks it a byte
>> at a time, off the end. 0xffffffb8 sizes it at 0 and loops forever.
>>
>> Sum in u64 so the check sees the real size, and reject a size the int
>> helpers cannot carry, since the walk advances by their return value.
>>
>> This is the per-section upper bound the later "len < sizeof(*foo)" guards
>> rely on; they are lower bounds only.
>>
>> Reported-by: sashiko-bot@kernel.org
>> Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=1
>> Fixes: 45b14a4ffcc1 ("efi: cper: Fix possible out-of-bounds access")
>> Assisted-by: Claude:claude-opus-4-8
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> I know I'm late to the discussion but maybe it would just be simpler to
> use check_add_overflow()?
> 
>> ---
>> v4:
>> - Do the arithmetic in u64 at the choke point instead of bounding the u32
>>   error_data_length against data_len, so the check no longer depends on the
>>   signed helpers in <acpi/ghes.h> behaving (Tony Luck). Bounding the u32
>>   still left a window when data_length itself was within a header of
>>   U32_MAX, and it did not stop acpi_hest_get_next() advancing by a wrapped
>>   int. The v3 "< 0" arm was dead either way, since data_len is unsigned and
>>   promoted the int back (Tony Luck, Shuai Xue).
>> - Dropped Alison's and Shuai's Reviewed-by; the check was reworked after
>>   they reviewed it.
>> ---
>>  drivers/firmware/efi/cper.c | 16 +++++++++++++---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
>> index 06b4fdb59917..ec092729cacc 100644
>> --- a/drivers/firmware/efi/cper.c
>> +++ b/drivers/firmware/efi/cper.c
>> @@ -752,7 +752,7 @@ EXPORT_SYMBOL_GPL(cper_estatus_check_header);
>>  int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
>>  {
>>  	struct acpi_hest_generic_data *gdata;
>> -	unsigned int data_len, record_size;
>> +	unsigned int data_len;
>>  	int rc;
>>  
>>  	rc = cper_estatus_check_header(estatus);
>> @@ -762,11 +762,21 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
>>  	data_len = estatus->data_length;
>>  
>>  	apei_estatus_for_each_section(estatus, gdata) {
>> +		u64 record_size;
>> +
>>  		if (acpi_hest_get_size(gdata) > data_len)
>>  			return -EINVAL;
>>  
>> -		record_size = acpi_hest_get_record_size(gdata);
>> -		if (record_size > data_len)
>> +		/*
>> +		 * acpi_hest_get_record_size() sums these as a signed int (see
>> +		 * <acpi/ghes.h>), which wraps small for a huge
>> +		 * error_data_length and slips past the check below. Sum in u64,
>> +		 * and reject what those helpers cannot carry, since the walk
>> +		 * advances by their return value.
>> +		 */
>> +		record_size = (u64)acpi_hest_get_size(gdata) +
>> +			      gdata->error_data_length;
> 
> I'm late to the game obviously and what you have works but could this have
> used some explicit overflow checking?  Something like

Not late at all.

> 
> 		if (check_add_overflow(acpi_hest_get_size(gdata),
> 				       gdata->error_data_length, &record_size))
> 			return -EINVAL;
> 
> 		if (record_size > data_len)
> 			return -EINVAL;
> 
> That uses the compiler __builtin_add_overflow() which checks if the infinite
> precision result of the sum of the parameters would have wrapped when written
> to the output one.
> 
> I think you could then drop the earlier check as well as
> record_size is at least as big as acpi_hest_get_size() and we know there
> was no wrap around.
> 

Yup I'll do that.

DJ

> 	
>> +		if (record_size > data_len || record_size > INT_MAX)
>>  			return -EINVAL;
>>  
>>  		data_len -= record_size;
> 

Re: [PATCH v4 03/13] ACPI: extlog: Validate elog record length before walking sections

[Dave Jiang]

On 8/24/26 3:28 PM, Jonathan Cameron wrote:
> On Mon, 24 Aug 2026 10:49:26 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> extlog_print() copies a fixed ELOG_ENTRY_LEN (4096) bytes from the elog
>> record into elog_buf, then walks the sections using the firmware-controlled
>> data_length. Nothing keeps data_length inside the buffer, so a malformed
>> record walks the section pointer past elog_buf and reads adjacent memory.
>> Unlike the GHES paths, extlog never calls cper_estatus_check().
>>
>> Reject a record longer than ELOG_ENTRY_LEN and run cper_estatus_check()
>> before walking the sections. The length test alone is not enough: a wrapped
>> length reads back short and passes it, which cper_estatus_check() catches
>> via the header check added earlier. Drop a malformed record with
>> NOTIFY_DONE and without MCE_HANDLED_EXTLOG, since extlog did not consume
>> it.
>>
>> Reported-by: sashiko-bot@kernel.org
>> Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=6
>> Fixes: f6ec01da40e4 ("ACPI: extlog: Handle multiple records")
>> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
>> Assisted-by: Claude:claude-opus-4-8
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>  drivers/acpi/acpi_extlog.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
>> index 7ad3b36013cc..9ad0052aa20c 100644
>> --- a/drivers/acpi/acpi_extlog.c
>> +++ b/drivers/acpi/acpi_extlog.c
>> @@ -208,6 +208,10 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
>>  
>>  	tmp = (struct acpi_hest_generic_status *)elog_buf;
>>  
>> +	/* Keep the firmware-controlled data_length inside elog_buf. */
>> +	if (cper_estatus_len(tmp) > ELOG_ENTRY_LEN || cper_estatus_check(tmp))
> 
> Why this order?  To me checking if we are in crazy world (overflow) before
> doing anything with the overflowed value makes more sense. So that would be swapping
> the two conditions.

Swapping it lets cper_estatus_check() walk the sections with data_length unbounded relative to elog_buf. elog_buf is a 4k buffer via kmalloc(ELOG_ENTRY_LEN). cper_estatus_check() iterates sections bounded by data_length, over a fixed kmalloc(ELOG_ENTRY_LEN) of 4096 bytes, reading gdata->revision at +20 and gdata->error_data_length at +24 each time. In the current order it only runs once cper_estatus_len() <= 4096 has passed, which caps data_length at 4076. Swapped, the only thing standing before the walk is cper_estatus_check_header(), and that admits data_length up to 4294967275.

Expand the comment to:

	/*
	 * Bound the length before cper_estatus_check() walks the sections: it
	 * iterates over data_length, which is not yet known to fit elog_buf.
	 * cper_estatus_check_header() then rejects a length that wrapped, which
	 * the bound cannot see.
	 */

DJ

> 
>> +		return NOTIFY_DONE;
>> +
>>  	if (!ras_userspace_consumers()) {
>>  		print_extlog_rcd(NULL, tmp, cpu);
>>  		goto out;
> 

Re: [PATCH v4 05/13] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

[Dave Jiang]

On 8/24/26 4:05 PM, Jonathan Cameron wrote:
> On Mon, 24 Aug 2026 10:49:28 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> extlog_print_pcie() casts pcie_err->aer_info straight to struct
>> aer_capability_regs *. That struct embeds struct pcie_tlp_log, whose
>> software-only header_len and flit fields sit at offset 84 - inside the
>> 96-byte aer_info buffer - so the cast fills them with raw firmware bytes.
>> pcie_print_tlp_log() uses both to bound a loop over dw[], and a large
>> header_len walks past the end of the array.
>>
>> Copy aer_info into a zeroed local struct aer_capability_regs and clear
>> header_len and flit before passing it on.
> 
> The existing code is very odd. It might be nice to clean it up more
> generally so that we can handle the rest of the aer_info buffer
> via a definition that matches the hardware spec. 
> 
> For now I'd be tempted to just copy the bit of the structure that
> is matching the spec - side effect being the rest ends up as zero
> including the two things you clear.
> 
> So just copy to first 4 dw of the header log. 

Ok will do.

> 
> Jonathan
> 
> 
> 
>>
>> Reported-by: sashiko-bot@kernel.org
>> Closes: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
>> Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
>> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
>> Assisted-by: Claude:claude-sonnet-4-6
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> v4:
>> - Dropped the now-pointless aer pointer alias.
>> ---
>>  drivers/acpi/acpi_extlog.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
>> index f6e3da4e13e7..6d5532ec0920 100644
>> --- a/drivers/acpi/acpi_extlog.c
>> +++ b/drivers/acpi/acpi_extlog.c
>> @@ -137,7 +137,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>>  			      int severity)
>>  {
>>  #ifdef ACPI_APEI_PCIEAER
>> -	struct aer_capability_regs *aer;
>> +	struct aer_capability_regs aer_regs = {};
>>  	struct pci_dev *pdev;
>>  	unsigned int devfn;
>>  	unsigned int bus;
>> @@ -149,7 +149,11 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>>  		return;
>>  
>>  	aer_severity = cper_severity_to_aer(severity);
>> -	aer = (struct aer_capability_regs *)pcie_err->aer_info;
>> +
>> +	memcpy(&aer_regs, pcie_err->aer_info, sizeof(pcie_err->aer_info));
>> +	aer_regs.header_log.header_len = 0;
>> +	aer_regs.header_log.flit = false;
>> +
>>  	domain = pcie_err->device_id.segment;
>>  	bus = pcie_err->device_id.bus;
>>  	devfn = PCI_DEVFN(pcie_err->device_id.device,
>> @@ -158,7 +162,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>>  	if (!pdev)
>>  		return;
>>  
>> -	pci_print_aer(pdev, aer_severity, aer);
>> +	pci_print_aer(pdev, aer_severity, &aer_regs);
>>  	pci_dev_put(pdev);
>>  #endif
>>  }
>