Linux ACPI
 help / color / mirror / Atom feed
* [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko
@ 2026-07-17 16:16 Dave Jiang
  2026-07-17 16:16 ` [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
                   ` (10 more replies)
  0 siblings, 11 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	Ben Cheatham

A collection of fixes for pre-existing issues reported by sashiko-bot while
reviewing patches.

The v1 posting fixed a first batch of these issues, and v2 grew the set as
sashiko-bot's review surfaced more problems in the same CPER/extlog paths.
sashiko-bot's review of v2 flagged yet more of the same class of bug in
neighbouring code, so v3 adds those as well: an out-of-range section length
that defeats cper_estatus_check(), the same unvalidated AER buffer handling
in ghes_handle_aer() that was already fixed for extlog, and an unbounded
walk of the extlog record.

1/10:  Bound the CXL event record copy to the firmware section length.
2/10:  Reject CPER records with an out-of-range error_data_length.
3/10:  Validate the CXL protocol error section length before the RAS cap copy.
4/10:  Avoid populating software AER metadata from the raw hardware buffer.
5/10:  Validate the PCIe error section length before payload access.
6/10:  Fix the CONFIG_ACPI_APEI_PCIEAER guard typo in extlog.c.
7/10:  Defer CXL protocol error handling to avoid a lock inversion.
8/10:  Validate the memory error section length before payload access.
9/10:  Bound the AER info copy and sanitize software metadata in ghes.c.
10/10: Validate the extlog record length before walking sections.

Note patch 2 touches drivers/firmware/efi/cper.c; it closes the length
validation hole at the shared choke point that the per-section guards in the
rest of the series rely on.

sashiko-bot's review of v2 also flagged a few related issues that are not
addressed here because they live in other subsystems.

  - cxl_cper_print_prot_err() in drivers/firmware/efi/cper_cxl.c uses the
    firmware-controlled 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 into a struct aer_capability_regs without clearing
    the software-only header_len/flit fields, the same class of issue as
    patches 4 and 9.

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/

Changes since v2
----------------
- Dropped the standalone spin_lock_irqsave() change; it is 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 bypass the check and 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 the CXL protocol error
  handling is deferred to the workqueue, before the typo fix that activates
  that code.

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


Dave Jiang (10):
  ACPI: APEI: GHES: Bound CXL event record copy to the firmware section
    length
  efi/cper: Reject CPER records with an out-of-range error_data_length
  ACPI: APEI: GHES: Validate CXL protocol error section length before
    RAS cap copy
  ACPI: extlog: Avoid populating software AER metadata from raw hardware
    buffer
  ACPI: extlog: Validate PCIe error section length before payload access
  ACPI: extlog: Defer CXL protocol error handling to avoid lock
    inversion
  ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
  ACPI: APEI: GHES: Validate memory error section length before payload
    access
  ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
  ACPI: extlog: Validate elog record length before walking sections

 drivers/acpi/acpi_extlog.c       | 47 ++++++++++++-------------
 drivers/acpi/apei/ghes.c         | 59 +++++++++++++++++++++++++-------
 drivers/acpi/apei/ghes_helpers.c | 21 +++++++++++-
 drivers/firmware/efi/cper.c      | 10 ++++++
 include/acpi/ghes.h              |  4 +++
 include/cxl/event.h              |  4 +--
 6 files changed, 106 insertions(+), 39 deletions(-)


base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
-- 
2.55.0


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

* [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
@ 2026-07-17 16:16 ` Dave Jiang
  2026-07-20 12:21   ` Hanjun Guo
                     ` (2 more replies)
  2026-07-17 16:16 ` [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length Dave Jiang
                   ` (9 subsequent siblings)
  10 siblings, 3 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot

sashiko-bot flagged a missing bounds check that allows an
out-of-bounds read of the firmware CPER section.

cxl_cper_post_event() copies a fixed sizeof(struct cxl_cper_event_rec)
out of the firmware CPER section without checking its length. 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")
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 3236a3ce79d6..a752e152a5a0 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -815,10 +815,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",
@@ -949,15 +954,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.55.0


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

* [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
  2026-07-17 16:16 ` [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
@ 2026-07-17 16:16 ` Dave Jiang
  2026-07-20 16:24   ` Alison Schofield
  2026-08-12 12:23   ` Shuai Xue
  2026-07-17 16:16 ` [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot

sashiko-bot flagged that the "len < sizeof(*rec)" guard added in this
series can be bypassed by an integer overflow in the shared length check.

cper_estatus_check() bounds firmware CPER data before the section
handlers in ghes_do_proc() run. It sizes each section with
acpi_hest_get_record_size(), which adds the firmware-controlled u32
error_data_length to the header size using signed int helpers in
<acpi/ghes.h>. A value like 0xffffffb9 sign-converts to a negative
number, wraps the record size small, and slips past the
"record_size > data_len" check. A section handler then copies a
fixed-size payload out of it and reads past the record.

Reject a section whose error_data_length is negative once sign-converted
or larger than the remaining data, before the size arithmetic runs.

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>
---
v3:
- New patch. Fix the signed error_data_length overflow that lets a
  crafted section bypass cper_estatus_check() and defeat the per-caller
  size guards (sashiko).
---
 drivers/firmware/efi/cper.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index 06b4fdb59917..99a86b2675e3 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -765,6 +765,16 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
 		if (acpi_hest_get_size(gdata) > data_len)
 			return -EINVAL;
 
+		/*
+		 * error_data_length reaches record_size below as a signed int
+		 * (see <acpi/ghes.h>), so a value with the sign bit set can
+		 * wrap record_size small and slip past the bound check. Reject
+		 * it before the arithmetic.
+		 */
+		if (acpi_hest_get_error_length(gdata) < 0 ||
+		    acpi_hest_get_error_length(gdata) > data_len)
+			return -EINVAL;
+
 		record_size = acpi_hest_get_record_size(gdata);
 		if (record_size > data_len)
 			return -EINVAL;
-- 
2.55.0


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

* [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
  2026-07-17 16:16 ` [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
  2026-07-17 16:16 ` [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length Dave Jiang
@ 2026-07-17 16:16 ` Dave Jiang
  2026-07-20 16:25   ` Alison Schofield
  2026-08-12 12:30   ` Shuai Xue
  2026-07-17 16:16 ` [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer Dave Jiang
                   ` (7 subsequent siblings)
  10 siblings, 2 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot, Ben Cheatham

sashiko-bot flagged an out-of-bounds read driven by an unvalidated
firmware dvsec_len.

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.

Extend cxl_cper_sec_prot_err_valid() to verify the section can hold the
header, and that the header, DVSEC and RAS Capability block all fit
within 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")
Assisted-by: Claude:claude-sonnet-4-6
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/acpi_extlog.c       |  7 ++++---
 drivers/acpi/apei/ghes.c         |  7 ++++---
 drivers/acpi/apei/ghes_helpers.c | 21 ++++++++++++++++++++-
 include/cxl/event.h              |  4 ++--
 4 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 7ad3b36013cc..06a944dadbc1 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -165,12 +165,12 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
 
 static void
 extlog_cxl_cper_handle_prot_err(struct cxl_cper_sec_prot_err *prot_err,
-				int severity)
+				int severity, u32 len)
 {
 #ifdef 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;
 
 	if (cxl_cper_setup_prot_err_work_data(&wd, prot_err, severity))
@@ -236,7 +236,8 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
 				acpi_hest_get_payload(gdata);
 
 			extlog_cxl_cper_handle_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 a752e152a5a0..17e4ef555292 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;
 
 static 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);
@@ -950,7 +950,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..d625ec98a24c 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,18 @@ int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err)
 		return -EINVAL;
 	}
 
+	/*
+	 * The RAS Capability block follows a firmware-controlled DVSEC of
+	 * dvsec_len bytes; verify it and the header fit the section.
+	 */
+	if (sizeof(*prot_err) + prot_err->dvsec_len +
+	    sizeof(struct cxl_ras_capability_regs) > len) {
+		pr_err_ratelimited(FW_WARN
+				   "CXL CPER prot err section too small (%u)\n",
+				   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/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.55.0


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

* [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
                   ` (2 preceding siblings ...)
  2026-07-17 16:16 ` [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
@ 2026-07-17 16:16 ` Dave Jiang
  2026-07-20 16:25   ` Alison Schofield
  2026-08-12 12:32   ` Shuai Xue
  2026-07-17 16:16 ` [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access Dave Jiang
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot

sashiko-bot flagged that casting the raw aer_info buffer fills
software-only AER metadata with hardware data, driving an out-of-bounds
read.

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

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
Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/acpi_extlog.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 06a944dadbc1..fbc88c584c06 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -137,6 +137,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
 			      int severity)
 {
 #ifdef ACPI_APEI_PCIEAER
+	struct aer_capability_regs aer_regs = {};
 	struct aer_capability_regs *aer;
 	struct pci_dev *pdev;
 	unsigned int devfn;
@@ -149,7 +150,12 @@ 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;
+	aer = &aer_regs;
+
 	domain = pcie_err->device_id.segment;
 	bus = pcie_err->device_id.bus;
 	devfn = PCI_DEVFN(pcie_err->device_id.device,
-- 
2.55.0


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

* [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
                   ` (3 preceding siblings ...)
  2026-07-17 16:16 ` [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer Dave Jiang
@ 2026-07-17 16:16 ` Dave Jiang
  2026-07-20 16:26   ` Alison Schofield
  2026-08-12 12:59   ` Shuai Xue
  2026-07-17 16:16 ` [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion Dave Jiang
                   ` (5 subsequent siblings)
  10 siblings, 2 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot

sashiko-bot flagged the missing section-length check before
extlog_print_pcie() touches the PCIe payload.

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 large enough for a struct cper_sec_pcie.
cper_estatus_check() keeps the read within the estatus block, but a
short section still lets stale adjacent bytes be treated as PCIe error
data. Reject a section too small to hold the record before touching any
field.

Reported-by: sashiko-bot@kernel.org
Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/acpi_extlog.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index fbc88c584c06..0c440d75d9a7 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 = {};
@@ -145,6 +145,9 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
 	int aer_severity;
 	int domain;
 
+	if (len < sizeof(*pcie_err))
+		return;
+
 	if (!(pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
 	      pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO))
 		return;
@@ -247,7 +250,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.55.0


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

* [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
                   ` (4 preceding siblings ...)
  2026-07-17 16:16 ` [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access Dave Jiang
@ 2026-07-17 16:16 ` Dave Jiang
  2026-07-20 16:26   ` Alison Schofield
  2026-08-12 13:06   ` Shuai Xue
  2026-07-17 16:16 ` [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
                   ` (4 subsequent siblings)
  10 siblings, 2 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot

sashiko-bot flagged an AB-BA deadlock between the PCI device_lock and
the MCE decoder chain rwsem.

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 locks in the opposite
order (device_lock held while mce_register_decode_chain() takes the
rwsem), so the two form an AB-BA deadlock.

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 use it from acpi_extlog.c instead of calling
cxl_cper_handle_prot_err() directly.

Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/acpi_extlog.c | 23 +++--------------------
 drivers/acpi/apei/ghes.c   |  5 +++--
 include/acpi/ghes.h        |  4 ++++
 3 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 0c440d75d9a7..ae79d090de33 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -172,23 +172,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, u32 len)
-{
-#ifdef ACPI_APEI_PCIEAER
-	struct cxl_cper_prot_err_work_data wd;
-
-	if (cxl_cper_sec_prot_err_valid(prot_err, len))
-		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)
 {
@@ -244,9 +227,9 @@ 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,
-							gdata->error_data_length);
+			cxl_cper_post_prot_err(prot_err,
+					       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 17e4ef555292..b8dbd99da47e 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, u32 len)
+void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
+			    int severity, u32 len)
 {
 #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..4dcbb2c30ea2 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -143,4 +143,8 @@ static inline int ghes_notify_sea(void) { return -ENOENT; }
 struct notifier_block;
 extern void ghes_register_report_chain(struct notifier_block *nb);
 extern void ghes_unregister_report_chain(struct notifier_block *nb);
+
+struct cxl_cper_sec_prot_err;
+void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
+			    int severity, u32 len);
 #endif /* GHES_H */
-- 
2.55.0


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

* [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
                   ` (5 preceding siblings ...)
  2026-07-17 16:16 ` [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion Dave Jiang
@ 2026-07-17 16:16 ` Dave Jiang
  2026-07-17 21:24   ` Dave Jiang
                     ` (2 more replies)
  2026-07-17 16:16 ` [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access Dave Jiang
                   ` (3 subsequent siblings)
  10 siblings, 3 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot, Ben Cheatham

sashiko-bot flagged that a Kconfig macro typo compiles out the extlog
CXL protocol error handling entirely.

The guard reads "#ifdef ACPI_APEI_PCIEAER" instead of
"#ifdef CONFIG_ACPI_APEI_PCIEAER", so the wrapped code is always compiled
out. Use the correct CONFIG_ prefixed symbol.

Reported-by: sashiko-bot@kernel.org
Fixes: 95350effc3ad ("ACPI: extlog: Trace CPER CXL Protocol Error Section")
Assisted-by: Claude:claude-opus-4-8
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 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 ae79d090de33..1fbaf134c04e 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 aer_capability_regs *aer;
 	struct pci_dev *pdev;
-- 
2.55.0


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

* [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
                   ` (6 preceding siblings ...)
  2026-07-17 16:16 ` [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
@ 2026-07-17 16:16 ` Dave Jiang
  2026-07-20 16:28   ` Alison Schofield
  2026-08-12 13:14   ` Shuai Xue
  2026-07-17 16:16 ` [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata Dave Jiang
                   ` (2 subsequent siblings)
  10 siblings, 2 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot

sashiko-bot flagged that the memory error length check was placed too
late, leaving the other payload consumers unprotected.

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. These read validation_bits
and physical_addr (offsets 0 and 16), so a shorter section reads past the
record.

Validate error_data_length once in ghes_do_proc(), before any consumer
runs, so every consumer is covered. Bound against struct
cper_sec_mem_err_old (the UEFI 2.1/2.2 layout) rather than the full
struct: the section length is authoritative, older firmware legitimately
emits the shorter record, and the trailing fields, though packed
unconditionally by cper_mem_err_pack(), are only acted on under
validation bits that a short record leaves clear. This matches the lower
bound already used in drivers/firmware/efi/cper.c.

A malformed sub-record no longer reaches ghes_handle_memory_failure().

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>
---
v3:
- Move the 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 instead of the full struct (sashiko).
---
 drivers/acpi/apei/ghes.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index b8dbd99da47e..1d2966a437bd 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -940,6 +940,17 @@ 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);
 
+			/*
+			 * Several consumers below dereference the record, so
+			 * check the length once here. Bound against the shorter
+			 * UEFI 2.1/2.2 layout that older firmware still emits;
+			 * the extra fields are only used under validation bits
+			 * such records leave clear.
+			 */
+			if (gdata->error_data_length <
+			    sizeof(struct cper_sec_mem_err_old))
+				continue;
+
 			atomic_notifier_call_chain(&ghes_report_chain, sev, mem_err);
 
 			arch_apei_report_mem_error(sev, mem_err);
-- 
2.55.0


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

* [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
                   ` (7 preceding siblings ...)
  2026-07-17 16:16 ` [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access Dave Jiang
@ 2026-07-17 16:16 ` Dave Jiang
  2026-07-20 16:28   ` Alison Schofield
  2026-08-12 13:16   ` Shuai Xue
  2026-07-17 16:16 ` [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections Dave Jiang
  2026-07-21 11:33 ` [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Rafael J. Wysocki (Intel)
  10 siblings, 2 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot

sashiko-bot flagged that ghes_handle_aer() has the same unvalidated
AER buffer handling plus a 4-byte over-read.

ghes_handle_aer() copies sizeof(struct aer_capability_regs) from the
fixed 96-byte pcie_err->aer_info, reading past the section since the
struct is larger. It also fills the software-only header_len and flit
fields of the embedded struct pcie_tlp_log from raw firmware bytes;
pcie_print_tlp_log() uses them to bound a loop over dw[], so a large
value walks past the array. There is also no check that the section can
hold a struct cper_sec_pcie.

Validate error_data_length, zero the destination, bound the copy to the
96-byte source, and clear header_len and flit. This mirrors 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")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v3:
- New patch. sashiko's review of v2 flagged the same unvalidated AER
  buffer handling in ghes_handle_aer() that v2 fixed for extlog.
---
 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 1d2966a437bd..bd53509dcab3 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));
+
+		/*
+		 * aer_info is a fixed 96-byte buffer, smaller than struct
+		 * aer_capability_regs, so bound the copy to the source. Clear
+		 * the software-only header_len and flit fields afterwards so
+		 * firmware bytes 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.55.0


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

* [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
                   ` (8 preceding siblings ...)
  2026-07-17 16:16 ` [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata Dave Jiang
@ 2026-07-17 16:16 ` Dave Jiang
  2026-07-20 16:29   ` Alison Schofield
  2026-08-12 13:18   ` Shuai Xue
  2026-07-21 11:33 ` [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Rafael J. Wysocki (Intel)
  10 siblings, 2 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 16:16 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot

sashiko-bot flagged a missing bounds check on data_length that allows
an out-of-bounds read of elog_buf.

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 checks that data_length stays
within the buffer, so a malformed record can walk the section pointer
past elog_buf and read adjacent memory.

Unlike the GHES paths, extlog never calls cper_estatus_check(). Reject a
record whose length exceeds ELOG_ENTRY_LEN and run cper_estatus_check()
before walking the sections.

A malformed record is now dropped with NOTIFY_DONE without setting
MCE_HANDLED_EXTLOG, reflecting that 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")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v3:
- New patch. sashiko's review of v2 pointed out extlog walks the elog
  sections without the cper_estatus_check() bound the GHES paths use.
---
 drivers/acpi/acpi_extlog.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 1fbaf134c04e..30863fd3f5b0 100644
--- a/drivers/acpi/acpi_extlog.c
+++ b/drivers/acpi/acpi_extlog.c
@@ -200,6 +200,13 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
 
 	tmp = (struct acpi_hest_generic_status *)elog_buf;
 
+	/*
+	 * data_length is firmware controlled, so make sure the record and its
+	 * sections stay within elog_buf before anything walks them.
+	 */
+	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.55.0


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

* Re: [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
  2026-07-17 16:16 ` [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
@ 2026-07-17 21:24   ` Dave Jiang
  2026-07-20 16:27   ` Alison Schofield
  2026-08-12 13:10   ` Shuai Xue
  2 siblings, 0 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-17 21:24 UTC (permalink / raw)
  To: linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai, terry.bowman,
	sashiko-bot, Ben Cheatham



On 7/17/26 9:16 AM, Dave Jiang wrote:
> sashiko-bot flagged that a Kconfig macro typo compiles out the extlog
> CXL protocol error handling entirely.

Sashiko flagged this. It should say:
sashiko-bot flagged that a Kconfig macro typo compiles out the extlog
PCIe AER handling entirely.

> 
> The guard reads "#ifdef ACPI_APEI_PCIEAER" instead of
> "#ifdef CONFIG_ACPI_APEI_PCIEAER", so the wrapped code is always compiled
> out. Use the correct CONFIG_ prefixed symbol.
> 
> Reported-by: sashiko-bot@kernel.org
> Fixes: 95350effc3ad ("ACPI: extlog: Trace CPER CXL Protocol Error Section")

Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")

If there are no other changes to the series, maybe the maintainer can fix this up?

DJ

> Assisted-by: Claude:claude-opus-4-8
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  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 ae79d090de33..1fbaf134c04e 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 aer_capability_regs *aer;
>  	struct pci_dev *pdev;


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

* Re: [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length
  2026-07-17 16:16 ` [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
@ 2026-07-20 12:21   ` Hanjun Guo
  2026-07-20 16:15     ` Dave Jiang
  2026-07-20 16:23   ` Alison Schofield
  2026-08-12 12:10   ` Shuai Xue
  2 siblings, 1 reply; 37+ messages in thread
From: Hanjun Guo @ 2026-07-20 12:21 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, mchehab, xueshuai, terry.bowman,
	sashiko-bot

On 2026/7/18 0:16, Dave Jiang wrote:
> sashiko-bot flagged a missing bounds check that allows an
> out-of-bounds read of the firmware CPER section.
> 
> cxl_cper_post_event() copies a fixed sizeof(struct cxl_cper_event_rec)
> out of the firmware CPER section without checking its length. 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")
> 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 3236a3ce79d6..a752e152a5a0 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -815,10 +815,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",
> @@ -949,15 +954,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);

There are multi places in this patch set doing the same thing, I was
thinking if we can do the check in on place but we are facing different
row error data, any good idea?

Thanks
Hanjun

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

* Re: [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length
  2026-07-20 12:21   ` Hanjun Guo
@ 2026-07-20 16:15     ` Dave Jiang
  0 siblings, 0 replies; 37+ messages in thread
From: Dave Jiang @ 2026-07-20 16:15 UTC (permalink / raw)
  To: Hanjun Guo, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, mchehab, xueshuai, terry.bowman,
	sashiko-bot



On 7/20/26 5:21 AM, Hanjun Guo wrote:
> On 2026/7/18 0:16, Dave Jiang wrote:
>> sashiko-bot flagged a missing bounds check that allows an
>> out-of-bounds read of the firmware CPER section.
>>
>> cxl_cper_post_event() copies a fixed sizeof(struct cxl_cper_event_rec)
>> out of the firmware CPER section without checking its length. 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")
>> 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 3236a3ce79d6..a752e152a5a0 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -815,10 +815,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",
>> @@ -949,15 +954,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);
> 
> There are multi places in this patch set doing the same thing, I was
> thinking if we can do the check in on place but we are facing different
> row error data, any good idea?

There's no easy single place to do this as the reason you stated.

> 
> Thanks
> Hanjun


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

* Re: [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length
  2026-07-17 16:16 ` [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
  2026-07-20 12:21   ` Hanjun Guo
@ 2026-07-20 16:23   ` Alison Schofield
  2026-08-12 12:10   ` Shuai Xue
  2 siblings, 0 replies; 37+ messages in thread
From: Alison Schofield @ 2026-07-20 16:23 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, sashiko-bot

On Fri, Jul 17, 2026 at 09:16:38AM -0700, Dave Jiang wrote:
> sashiko-bot flagged a missing bounds check that allows an
> out-of-bounds read of the firmware CPER section.
> 
> cxl_cper_post_event() copies a fixed sizeof(struct cxl_cper_event_rec)
> out of the firmware CPER section without checking its length. 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")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

* Re: [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length
  2026-07-17 16:16 ` [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length Dave Jiang
@ 2026-07-20 16:24   ` Alison Schofield
  2026-08-12 12:23   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Alison Schofield @ 2026-07-20 16:24 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, sashiko-bot

On Fri, Jul 17, 2026 at 09:16:39AM -0700, Dave Jiang wrote:
> sashiko-bot flagged that the "len < sizeof(*rec)" guard added in this
> series can be bypassed by an integer overflow in the shared length check.
> 
> cper_estatus_check() bounds firmware CPER data before the section
> handlers in ghes_do_proc() run. It sizes each section with
> acpi_hest_get_record_size(), which adds the firmware-controlled u32
> error_data_length to the header size using signed int helpers in
> <acpi/ghes.h>. A value like 0xffffffb9 sign-converts to a negative
> number, wraps the record size small, and slips past the
> "record_size > data_len" check. A section handler then copies a
> fixed-size payload out of it and reads past the record.
> 
> Reject a section whose error_data_length is negative once sign-converted
> or larger than the remaining data, before the size arithmetic runs.
> 
> 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>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

* Re: [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy
  2026-07-17 16:16 ` [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
@ 2026-07-20 16:25   ` Alison Schofield
  2026-08-12 12:30   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Alison Schofield @ 2026-07-20 16:25 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, sashiko-bot, Ben Cheatham

On Fri, Jul 17, 2026 at 09:16:40AM -0700, Dave Jiang wrote:
> sashiko-bot flagged an out-of-bounds read driven by an unvalidated
> firmware dvsec_len.
> 
> 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.
> 
> Extend cxl_cper_sec_prot_err_valid() to verify the section can hold the
> header, and that the header, DVSEC and RAS Capability block all fit
> within 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")
> Assisted-by: Claude:claude-sonnet-4-6
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

* Re: [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer
  2026-07-17 16:16 ` [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer Dave Jiang
@ 2026-07-20 16:25   ` Alison Schofield
  2026-08-12 12:32   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Alison Schofield @ 2026-07-20 16:25 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, sashiko-bot

On Fri, Jul 17, 2026 at 09:16:41AM -0700, Dave Jiang wrote:
> sashiko-bot flagged that casting the raw aer_info buffer fills
> software-only AER metadata with hardware data, driving an out-of-bounds
> read.
> 
> extlog_print_pcie() casts pcie_err->aer_info directly to struct
> aer_capability_regs *. That struct embeds struct pcie_tlp_log, whose
> software-only header_len and flit fields sit at offset 84, within the
> 96-byte aer_info buffer, so the cast fills them with raw hardware data.
> pcie_print_tlp_log() uses flit and header_len to bound a loop over the
> dw[] array, so a large header_len walks past its end.
> 
> 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
> Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
> Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

* Re: [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access
  2026-07-17 16:16 ` [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access Dave Jiang
@ 2026-07-20 16:26   ` Alison Schofield
  2026-08-12 12:59   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Alison Schofield @ 2026-07-20 16:26 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, sashiko-bot

On Fri, Jul 17, 2026 at 09:16:42AM -0700, Dave Jiang wrote:
> sashiko-bot flagged the missing section-length check before
> extlog_print_pcie() touches the PCIe payload.
> 
> 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 large enough for a struct cper_sec_pcie.
> cper_estatus_check() keeps the read within the estatus block, but a
> short section still lets stale adjacent bytes be treated as PCIe error
> data. Reject a section too small to hold the record before touching any
> field.
> 
> Reported-by: sashiko-bot@kernel.org
> Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

* Re: [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion
  2026-07-17 16:16 ` [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion Dave Jiang
@ 2026-07-20 16:26   ` Alison Schofield
  2026-08-12 13:06   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Alison Schofield @ 2026-07-20 16:26 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, sashiko-bot

On Fri, Jul 17, 2026 at 09:16:43AM -0700, Dave Jiang wrote:
> sashiko-bot flagged an AB-BA deadlock between the PCI device_lock and
> the MCE decoder chain rwsem.
> 
> 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 locks in the opposite
> order (device_lock held while mce_register_decode_chain() takes the
> rwsem), so the two form an AB-BA deadlock.
> 
> 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 use it from acpi_extlog.c instead of calling
> cxl_cper_handle_prot_err() directly.
> 
> Reported-by: sashiko-bot@kernel.org
> Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

* Re: [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
  2026-07-17 16:16 ` [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
  2026-07-17 21:24   ` Dave Jiang
@ 2026-07-20 16:27   ` Alison Schofield
  2026-08-12 13:10   ` Shuai Xue
  2 siblings, 0 replies; 37+ messages in thread
From: Alison Schofield @ 2026-07-20 16:27 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, sashiko-bot, Ben Cheatham

On Fri, Jul 17, 2026 at 09:16:44AM -0700, Dave Jiang wrote:
> sashiko-bot flagged that a Kconfig macro typo compiles out the extlog
> CXL protocol error handling entirely.
> 
> The guard reads "#ifdef ACPI_APEI_PCIEAER" instead of
> "#ifdef CONFIG_ACPI_APEI_PCIEAER", so the wrapped code is always compiled
> out. Use the correct CONFIG_ prefixed symbol.
> 
> Reported-by: sashiko-bot@kernel.org
> Fixes: 95350effc3ad ("ACPI: extlog: Trace CPER CXL Protocol Error Section")
> Assisted-by: Claude:claude-opus-4-8
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

* Re: [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access
  2026-07-17 16:16 ` [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access Dave Jiang
@ 2026-07-20 16:28   ` Alison Schofield
  2026-08-12 13:14   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Alison Schofield @ 2026-07-20 16:28 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, sashiko-bot

On Fri, Jul 17, 2026 at 09:16:45AM -0700, Dave Jiang wrote:
> sashiko-bot flagged that the memory error length check was placed too
> late, leaving the other payload consumers unprotected.
> 
> 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. These read validation_bits
> and physical_addr (offsets 0 and 16), so a shorter section reads past the
> record.
> 
> Validate error_data_length once in ghes_do_proc(), before any consumer
> runs, so every consumer is covered. Bound against struct
> cper_sec_mem_err_old (the UEFI 2.1/2.2 layout) rather than the full
> struct: the section length is authoritative, older firmware legitimately
> emits the shorter record, and the trailing fields, though packed
> unconditionally by cper_mem_err_pack(), are only acted on under
> validation bits that a short record leaves clear. This matches the lower
> bound already used in drivers/firmware/efi/cper.c.
> 
> A malformed sub-record no longer reaches ghes_handle_memory_failure().
> 
> 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>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

* Re: [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
  2026-07-17 16:16 ` [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata Dave Jiang
@ 2026-07-20 16:28   ` Alison Schofield
  2026-08-12 13:16   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Alison Schofield @ 2026-07-20 16:28 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, sashiko-bot

On Fri, Jul 17, 2026 at 09:16:46AM -0700, Dave Jiang wrote:
> sashiko-bot flagged that ghes_handle_aer() has the same unvalidated
> AER buffer handling plus a 4-byte over-read.
> 
> ghes_handle_aer() copies sizeof(struct aer_capability_regs) from the
> fixed 96-byte pcie_err->aer_info, reading past the section since the
> struct is larger. It also fills the software-only header_len and flit
> fields of the embedded struct pcie_tlp_log from raw firmware bytes;
> pcie_print_tlp_log() uses them to bound a loop over dw[], so a large
> value walks past the array. There is also no check that the section can
> hold a struct cper_sec_pcie.
> 
> Validate error_data_length, zero the destination, bound the copy to the
> 96-byte source, and clear header_len and flit. This mirrors 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")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

* Re: [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections
  2026-07-17 16:16 ` [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections Dave Jiang
@ 2026-07-20 16:29   ` Alison Schofield
  2026-08-12 13:18   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Alison Schofield @ 2026-07-20 16:29 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, sashiko-bot

On Fri, Jul 17, 2026 at 09:16:47AM -0700, Dave Jiang wrote:
> sashiko-bot flagged a missing bounds check on data_length that allows
> an out-of-bounds read of elog_buf.
> 
> 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 checks that data_length stays
> within the buffer, so a malformed record can walk the section pointer
> past elog_buf and read adjacent memory.
> 
> Unlike the GHES paths, extlog never calls cper_estatus_check(). Reject a
> record whose length exceeds ELOG_ENTRY_LEN and run cper_estatus_check()
> before walking the sections.
> 
> A malformed record is now dropped with NOTIFY_DONE without setting
> MCE_HANDLED_EXTLOG, reflecting that 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")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

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

* Re: [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko
  2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
                   ` (9 preceding siblings ...)
  2026-07-17 16:16 ` [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections Dave Jiang
@ 2026-07-21 11:33 ` Rafael J. Wysocki (Intel)
  2026-07-21 15:06   ` Dave Jiang
  10 siblings, 1 reply; 37+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-07-21 11:33 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-acpi, linux-cxl, rafael, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, Ben Cheatham

On Fri, Jul 17, 2026 at 6:16 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> A collection of fixes for pre-existing issues reported by sashiko-bot while
> reviewing patches.
>
> The v1 posting fixed a first batch of these issues, and v2 grew the set as
> sashiko-bot's review surfaced more problems in the same CPER/extlog paths.
> sashiko-bot's review of v2 flagged yet more of the same class of bug in
> neighbouring code, so v3 adds those as well: an out-of-range section length
> that defeats cper_estatus_check(), the same unvalidated AER buffer handling
> in ghes_handle_aer() that was already fixed for extlog, and an unbounded
> walk of the extlog record.
>
> 1/10:  Bound the CXL event record copy to the firmware section length.
> 2/10:  Reject CPER records with an out-of-range error_data_length.
> 3/10:  Validate the CXL protocol error section length before the RAS cap copy.
> 4/10:  Avoid populating software AER metadata from the raw hardware buffer.
> 5/10:  Validate the PCIe error section length before payload access.
> 6/10:  Fix the CONFIG_ACPI_APEI_PCIEAER guard typo in extlog.c.
> 7/10:  Defer CXL protocol error handling to avoid a lock inversion.
> 8/10:  Validate the memory error section length before payload access.
> 9/10:  Bound the AER info copy and sanitize software metadata in ghes.c.
> 10/10: Validate the extlog record length before walking sections.
>
> Note patch 2 touches drivers/firmware/efi/cper.c; it closes the length
> validation hole at the shared choke point that the per-section guards in the
> rest of the series rely on.
>
> sashiko-bot's review of v2 also flagged a few related issues that are not
> addressed here because they live in other subsystems.
>
>   - cxl_cper_print_prot_err() in drivers/firmware/efi/cper_cxl.c uses the
>     firmware-controlled 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 into a struct aer_capability_regs without clearing
>     the software-only header_len/flit fields, the same class of issue as
>     patches 4 and 9.
>
> 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/
>
> Changes since v2
> ----------------
> - Dropped the standalone spin_lock_irqsave() change; it is 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 bypass the check and 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 the CXL protocol error
>   handling is deferred to the workqueue, before the typo fix that activates
>   that code.
>
> Changes since v1
> ----------------
> - See the v2 posting for the full v1 -> v2 changelog.
>
>
> Dave Jiang (10):
>   ACPI: APEI: GHES: Bound CXL event record copy to the firmware section
>     length
>   efi/cper: Reject CPER records with an out-of-range error_data_length
>   ACPI: APEI: GHES: Validate CXL protocol error section length before
>     RAS cap copy
>   ACPI: extlog: Avoid populating software AER metadata from raw hardware
>     buffer
>   ACPI: extlog: Validate PCIe error section length before payload access
>   ACPI: extlog: Defer CXL protocol error handling to avoid lock
>     inversion
>   ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
>   ACPI: APEI: GHES: Validate memory error section length before payload
>     access
>   ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
>   ACPI: extlog: Validate elog record length before walking sections
>
>  drivers/acpi/acpi_extlog.c       | 47 ++++++++++++-------------
>  drivers/acpi/apei/ghes.c         | 59 +++++++++++++++++++++++++-------
>  drivers/acpi/apei/ghes_helpers.c | 21 +++++++++++-
>  drivers/firmware/efi/cper.c      | 10 ++++++
>  include/acpi/ghes.h              |  4 +++
>  include/cxl/event.h              |  4 +--
>  6 files changed, 106 insertions(+), 39 deletions(-)

All of this looks reasonable to me, but it really is material for Tony
to look at.

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

* Re: [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko
  2026-07-21 11:33 ` [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Rafael J. Wysocki (Intel)
@ 2026-07-21 15:06   ` Dave Jiang
  2026-08-14 20:16     ` Rafael J. Wysocki (Intel)
  0 siblings, 1 reply; 37+ messages in thread
From: Dave Jiang @ 2026-07-21 15:06 UTC (permalink / raw)
  To: Rafael J. Wysocki (Intel)
  Cc: linux-acpi, linux-cxl, tony.luck, bp, guohanjun, mchehab,
	xueshuai, terry.bowman, Ben Cheatham



On 7/21/26 4:33 AM, Rafael J. Wysocki (Intel) wrote:
> On Fri, Jul 17, 2026 at 6:16 PM Dave Jiang <dave.jiang@intel.com> wrote:
>>
>> A collection of fixes for pre-existing issues reported by sashiko-bot while
>> reviewing patches.
>>
>> The v1 posting fixed a first batch of these issues, and v2 grew the set as
>> sashiko-bot's review surfaced more problems in the same CPER/extlog paths.
>> sashiko-bot's review of v2 flagged yet more of the same class of bug in
>> neighbouring code, so v3 adds those as well: an out-of-range section length
>> that defeats cper_estatus_check(), the same unvalidated AER buffer handling
>> in ghes_handle_aer() that was already fixed for extlog, and an unbounded
>> walk of the extlog record.
>>
>> 1/10:  Bound the CXL event record copy to the firmware section length.
>> 2/10:  Reject CPER records with an out-of-range error_data_length.
>> 3/10:  Validate the CXL protocol error section length before the RAS cap copy.
>> 4/10:  Avoid populating software AER metadata from the raw hardware buffer.
>> 5/10:  Validate the PCIe error section length before payload access.
>> 6/10:  Fix the CONFIG_ACPI_APEI_PCIEAER guard typo in extlog.c.
>> 7/10:  Defer CXL protocol error handling to avoid a lock inversion.
>> 8/10:  Validate the memory error section length before payload access.
>> 9/10:  Bound the AER info copy and sanitize software metadata in ghes.c.
>> 10/10: Validate the extlog record length before walking sections.
>>
>> Note patch 2 touches drivers/firmware/efi/cper.c; it closes the length
>> validation hole at the shared choke point that the per-section guards in the
>> rest of the series rely on.
>>
>> sashiko-bot's review of v2 also flagged a few related issues that are not
>> addressed here because they live in other subsystems.
>>
>>   - cxl_cper_print_prot_err() in drivers/firmware/efi/cper_cxl.c uses the
>>     firmware-controlled 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 into a struct aer_capability_regs without clearing
>>     the software-only header_len/flit fields, the same class of issue as
>>     patches 4 and 9.
>>
>> 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/
>>
>> Changes since v2
>> ----------------
>> - Dropped the standalone spin_lock_irqsave() change; it is 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 bypass the check and 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 the CXL protocol error
>>   handling is deferred to the workqueue, before the typo fix that activates
>>   that code.
>>
>> Changes since v1
>> ----------------
>> - See the v2 posting for the full v1 -> v2 changelog.
>>
>>
>> Dave Jiang (10):
>>   ACPI: APEI: GHES: Bound CXL event record copy to the firmware section
>>     length
>>   efi/cper: Reject CPER records with an out-of-range error_data_length
>>   ACPI: APEI: GHES: Validate CXL protocol error section length before
>>     RAS cap copy
>>   ACPI: extlog: Avoid populating software AER metadata from raw hardware
>>     buffer
>>   ACPI: extlog: Validate PCIe error section length before payload access
>>   ACPI: extlog: Defer CXL protocol error handling to avoid lock
>>     inversion
>>   ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
>>   ACPI: APEI: GHES: Validate memory error section length before payload
>>     access
>>   ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
>>   ACPI: extlog: Validate elog record length before walking sections
>>
>>  drivers/acpi/acpi_extlog.c       | 47 ++++++++++++-------------
>>  drivers/acpi/apei/ghes.c         | 59 +++++++++++++++++++++++++-------
>>  drivers/acpi/apei/ghes_helpers.c | 21 +++++++++++-
>>  drivers/firmware/efi/cper.c      | 10 ++++++
>>  include/acpi/ghes.h              |  4 +++
>>  include/cxl/event.h              |  4 +--
>>  6 files changed, 106 insertions(+), 39 deletions(-)
> 
> All of this looks reasonable to me, but it really is material for Tony
> to look at.

Thanks Rafael. I've asked Tony to take a look. Looks like he's on vacation so hopefully soon. 


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

* Re: [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length
  2026-07-17 16:16 ` [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
  2026-07-20 12:21   ` Hanjun Guo
  2026-07-20 16:23   ` Alison Schofield
@ 2026-08-12 12:10   ` Shuai Xue
  2 siblings, 0 replies; 37+ messages in thread
From: Shuai Xue @ 2026-08-12 12:10 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, terry.bowman,
	sashiko-bot



On 7/18/26 12:16 AM, Dave Jiang wrote:
> sashiko-bot flagged a missing bounds check that allows an
> out-of-bounds read of the firmware CPER section.
> 
> cxl_cper_post_event() copies a fixed sizeof(struct cxl_cper_event_rec)
> out of the firmware CPER section without checking its length. 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")
> 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 3236a3ce79d6..a752e152a5a0 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -815,10 +815,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",
> @@ -949,15 +954,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);
>   


Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

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

* Re: [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length
  2026-07-17 16:16 ` [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length Dave Jiang
  2026-07-20 16:24   ` Alison Schofield
@ 2026-08-12 12:23   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Shuai Xue @ 2026-08-12 12:23 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, terry.bowman,
	sashiko-bot



On 7/18/26 12:16 AM, Dave Jiang wrote:
> sashiko-bot flagged that the "len < sizeof(*rec)" guard added in this
> series can be bypassed by an integer overflow in the shared length check.
> 
> cper_estatus_check() bounds firmware CPER data before the section
> handlers in ghes_do_proc() run. It sizes each section with
> acpi_hest_get_record_size(), which adds the firmware-controlled u32
> error_data_length to the header size using signed int helpers in
> <acpi/ghes.h>. A value like 0xffffffb9 sign-converts to a negative
> number, wraps the record size small, and slips past the
> "record_size > data_len" check. A section handler then copies a
> fixed-size payload out of it and reads past the record.
> 
> Reject a section whose error_data_length is negative once sign-converted
> or larger than the remaining data, before the size arithmetic runs.
> 
> 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>
> ---
> v3:
> - New patch. Fix the signed error_data_length overflow that lets a
>    crafted section bypass cper_estatus_check() and defeat the per-caller
>    size guards (sashiko).
> ---
>   drivers/firmware/efi/cper.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index 06b4fdb59917..99a86b2675e3 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -765,6 +765,16 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
>   		if (acpi_hest_get_size(gdata) > data_len)
>   			return -EINVAL;
>   
> +		/*
> +		 * error_data_length reaches record_size below as a signed int
> +		 * (see <acpi/ghes.h>), so a value with the sign bit set can
> +		 * wrap record_size small and slip past the bound check. Reject
> +		 * it before the arithmetic.
> +		 */
> +		if (acpi_hest_get_error_length(gdata) < 0 ||
> +		    acpi_hest_get_error_length(gdata) > data_len)
> +			return -EINVAL;
> +
>   		record_size = acpi_hest_get_record_size(gdata);
>   		if (record_size > data_len)
>   			return -EINVAL;

Minor: Could pull the helper result into a local, it's called twice.

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai


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

* Re: [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy
  2026-07-17 16:16 ` [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
  2026-07-20 16:25   ` Alison Schofield
@ 2026-08-12 12:30   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Shuai Xue @ 2026-08-12 12:30 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, terry.bowman,
	sashiko-bot, Ben Cheatham



On 7/18/26 12:16 AM, Dave Jiang wrote:
> sashiko-bot flagged an out-of-bounds read driven by an unvalidated
> firmware dvsec_len.
> 
> 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.
> 
> Extend cxl_cper_sec_prot_err_valid() to verify the section can hold the
> header, and that the header, DVSEC and RAS Capability block all fit
> within 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")
> Assisted-by: Claude:claude-sonnet-4-6
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   drivers/acpi/acpi_extlog.c       |  7 ++++---
>   drivers/acpi/apei/ghes.c         |  7 ++++---
>   drivers/acpi/apei/ghes_helpers.c | 21 ++++++++++++++++++++-
>   include/cxl/event.h              |  4 ++--
>   4 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 7ad3b36013cc..06a944dadbc1 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -165,12 +165,12 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>   
>   static void
>   extlog_cxl_cper_handle_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> -				int severity)
> +				int severity, u32 len)
>   {
>   #ifdef 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;
>   
>   	if (cxl_cper_setup_prot_err_work_data(&wd, prot_err, severity))
> @@ -236,7 +236,8 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
>   				acpi_hest_get_payload(gdata);
>   
>   			extlog_cxl_cper_handle_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 a752e152a5a0..17e4ef555292 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;
>   
>   static 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);
> @@ -950,7 +950,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..d625ec98a24c 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,18 @@ int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err)
>   		return -EINVAL;
>   	}
>   
> +	/*
> +	 * The RAS Capability block follows a firmware-controlled DVSEC of
> +	 * dvsec_len bytes; verify it and the header fit the section.
> +	 */
> +	if (sizeof(*prot_err) + prot_err->dvsec_len +
> +	    sizeof(struct cxl_ras_capability_regs) > len) {
> +		pr_err_ratelimited(FW_WARN
> +				   "CXL CPER prot err section too small (%u)\n",
> +				   len);

Nit, this prints the same message as the header-too-small case above, so
the log can't tell them apart. Maybe include dvsec_len here.

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

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

* Re: [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer
  2026-07-17 16:16 ` [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer Dave Jiang
  2026-07-20 16:25   ` Alison Schofield
@ 2026-08-12 12:32   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Shuai Xue @ 2026-08-12 12:32 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, terry.bowman,
	sashiko-bot



On 7/18/26 12:16 AM, Dave Jiang wrote:
> sashiko-bot flagged that casting the raw aer_info buffer fills
> software-only AER metadata with hardware data, driving an out-of-bounds
> read.
> 
> extlog_print_pcie() casts pcie_err->aer_info directly to struct
> aer_capability_regs *. That struct embeds struct pcie_tlp_log, whose
> software-only header_len and flit fields sit at offset 84, within the
> 96-byte aer_info buffer, so the cast fills them with raw hardware data.
> pcie_print_tlp_log() uses flit and header_len to bound a loop over the
> dw[] array, so a large header_len walks past its end.
> 
> 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
> Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
> Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   drivers/acpi/acpi_extlog.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 06a944dadbc1..fbc88c584c06 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -137,6 +137,7 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>   			      int severity)
>   {
>   #ifdef ACPI_APEI_PCIEAER
> +	struct aer_capability_regs aer_regs = {};
>   	struct aer_capability_regs *aer;
>   	struct pci_dev *pdev;
>   	unsigned int devfn;
> @@ -149,7 +150,12 @@ 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;
> +	aer = &aer_regs;
> +
>   	domain = pcie_err->device_id.segment;
>   	bus = pcie_err->device_id.bus;
>   	devfn = PCI_DEVFN(pcie_err->device_id.device,

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

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

* Re: [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access
  2026-07-17 16:16 ` [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access Dave Jiang
  2026-07-20 16:26   ` Alison Schofield
@ 2026-08-12 12:59   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Shuai Xue @ 2026-08-12 12:59 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, terry.bowman,
	sashiko-bot



On 7/18/26 12:16 AM, Dave Jiang wrote:
> sashiko-bot flagged the missing section-length check before
> extlog_print_pcie() touches the PCIe payload.
> 
> 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 large enough for a struct cper_sec_pcie.
> cper_estatus_check() keeps the read within the estatus block, but a
> short section still lets stale adjacent bytes be treated as PCIe error
> data. Reject a section too small to hold the record before touching any
> field.
> 
> Reported-by: sashiko-bot@kernel.org
> Fixes: e778ffefa34d ("ACPI: extlog: Trace CPER PCI Express Error Section")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   drivers/acpi/acpi_extlog.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index fbc88c584c06..0c440d75d9a7 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 = {};
> @@ -145,6 +145,9 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
>   	int aer_severity;
>   	int domain;
>   
> +	if (len < sizeof(*pcie_err))
> +		return;

Minor: The other length rejections this series adds (patch 1 and patch 3)
both print an FW_WARN message, but this one returns silently. Since
the point is to catch firmware emitting truncated PCIe sections, a
ratelimited warning here would match and make the problem visible.

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai


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

* Re: [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion
  2026-07-17 16:16 ` [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion Dave Jiang
  2026-07-20 16:26   ` Alison Schofield
@ 2026-08-12 13:06   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Shuai Xue @ 2026-08-12 13:06 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, terry.bowman,
	sashiko-bot



On 7/18/26 12:16 AM, Dave Jiang wrote:
> sashiko-bot flagged an AB-BA deadlock between the PCI device_lock and
> the MCE decoder chain rwsem.
> 
> 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 locks in the opposite
> order (device_lock held while mce_register_decode_chain() takes the
> rwsem), so the two form an AB-BA deadlock.
> 
> 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 use it from acpi_extlog.c instead of calling
> cxl_cper_handle_prot_err() directly.
> 
> Reported-by: sashiko-bot@kernel.org
> Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   drivers/acpi/acpi_extlog.c | 23 +++--------------------
>   drivers/acpi/apei/ghes.c   |  5 +++--
>   include/acpi/ghes.h        |  4 ++++
>   3 files changed, 10 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 0c440d75d9a7..ae79d090de33 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -172,23 +172,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, u32 len)
> -{
> -#ifdef ACPI_APEI_PCIEAER
> -	struct cxl_cper_prot_err_work_data wd;
> -
> -	if (cxl_cper_sec_prot_err_valid(prot_err, len))
> -		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)
>   {
> @@ -244,9 +227,9 @@ 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,
> -							gdata->error_data_length);
> +			cxl_cper_post_prot_err(prot_err,
> +					       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 17e4ef555292..b8dbd99da47e 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, u32 len)
> +void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> +			    int severity, u32 len)
>   {
>   #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..4dcbb2c30ea2 100644
> --- a/include/acpi/ghes.h
> +++ b/include/acpi/ghes.h
> @@ -143,4 +143,8 @@ static inline int ghes_notify_sea(void) { return -ENOENT; }
>   struct notifier_block;
>   extern void ghes_register_report_chain(struct notifier_block *nb);
>   extern void ghes_unregister_report_chain(struct notifier_block *nb);
> +
> +struct cxl_cper_sec_prot_err;
> +void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> +			    int severity, u32 len);
>   #endif /* GHES_H */

The declaration lands outside the CONFIG_ACPI_APEI_GHES section,
but the definition only exists in ghes.c, which is not built when
ACPI_APEI_GHES is disabled. ACPI_EXTLOG doesn't depend on GHES, so
GHES=n + EXTLOG=m is a legal config and modpost will fail on the
missing symbol. Either move the declaration into the GHES block with
a stub in the #else, or add the Kconfig dependency.

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

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

* Re: [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
  2026-07-17 16:16 ` [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
  2026-07-17 21:24   ` Dave Jiang
  2026-07-20 16:27   ` Alison Schofield
@ 2026-08-12 13:10   ` Shuai Xue
  2 siblings, 0 replies; 37+ messages in thread
From: Shuai Xue @ 2026-08-12 13:10 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, terry.bowman,
	sashiko-bot, Ben Cheatham



On 7/18/26 12:16 AM, Dave Jiang wrote:
> sashiko-bot flagged that a Kconfig macro typo compiles out the extlog
> CXL protocol error handling entirely.
> 
> The guard reads "#ifdef ACPI_APEI_PCIEAER" instead of
> "#ifdef CONFIG_ACPI_APEI_PCIEAER", so the wrapped code is always compiled
> out. Use the correct CONFIG_ prefixed symbol.
> 
> Reported-by: sashiko-bot@kernel.org
> Fixes: 95350effc3ad ("ACPI: extlog: Trace CPER CXL Protocol Error Section")
> Assisted-by: Claude:claude-opus-4-8
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   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 ae79d090de33..1fbaf134c04e 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 aer_capability_regs *aer;
>   	struct pci_dev *pdev;

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

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

* Re: [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access
  2026-07-17 16:16 ` [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access Dave Jiang
  2026-07-20 16:28   ` Alison Schofield
@ 2026-08-12 13:14   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Shuai Xue @ 2026-08-12 13:14 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, terry.bowman,
	sashiko-bot



On 7/18/26 12:16 AM, Dave Jiang wrote:
> sashiko-bot flagged that the memory error length check was placed too
> late, leaving the other payload consumers unprotected.
> 
> 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. These read validation_bits
> and physical_addr (offsets 0 and 16), so a shorter section reads past the
> record.
> 
> Validate error_data_length once in ghes_do_proc(), before any consumer
> runs, so every consumer is covered. Bound against struct
> cper_sec_mem_err_old (the UEFI 2.1/2.2 layout) rather than the full
> struct: the section length is authoritative, older firmware legitimately
> emits the shorter record, and the trailing fields, though packed
> unconditionally by cper_mem_err_pack(), are only acted on under
> validation bits that a short record leaves clear. This matches the lower
> bound already used in drivers/firmware/efi/cper.c.
> 
> A malformed sub-record no longer reaches ghes_handle_memory_failure().
> 
> 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>
> ---
> v3:
> - Move the 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 instead of the full struct (sashiko).
> ---
>   drivers/acpi/apei/ghes.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index b8dbd99da47e..1d2966a437bd 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -940,6 +940,17 @@ 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);
>   
> +			/*
> +			 * Several consumers below dereference the record, so
> +			 * check the length once here. Bound against the shorter
> +			 * UEFI 2.1/2.2 layout that older firmware still emits;
> +			 * the extra fields are only used under validation bits
> +			 * such records leave clear.
> +			 */
> +			if (gdata->error_data_length <
> +			    sizeof(struct cper_sec_mem_err_old))
> +				continue;
> +
>   			atomic_notifier_call_chain(&ghes_report_chain, sev, mem_err);
>   
>   			arch_apei_report_mem_error(sev, mem_err);

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

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

* Re: [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
  2026-07-17 16:16 ` [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata Dave Jiang
  2026-07-20 16:28   ` Alison Schofield
@ 2026-08-12 13:16   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Shuai Xue @ 2026-08-12 13:16 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, terry.bowman,
	sashiko-bot



On 7/18/26 12:16 AM, Dave Jiang wrote:
> sashiko-bot flagged that ghes_handle_aer() has the same unvalidated
> AER buffer handling plus a 4-byte over-read.
> 
> ghes_handle_aer() copies sizeof(struct aer_capability_regs) from the
> fixed 96-byte pcie_err->aer_info, reading past the section since the
> struct is larger. It also fills the software-only header_len and flit
> fields of the embedded struct pcie_tlp_log from raw firmware bytes;
> pcie_print_tlp_log() uses them to bound a loop over dw[], so a large
> value walks past the array. There is also no check that the section can
> hold a struct cper_sec_pcie.
> 
> Validate error_data_length, zero the destination, bound the copy to the
> 96-byte source, and clear header_len and flit. This mirrors 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")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v3:
> - New patch. sashiko's review of v2 flagged the same unvalidated AER
>    buffer handling in ghes_handle_aer() that v2 fixed for extlog.
> ---
>   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 1d2966a437bd..bd53509dcab3 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));
> +
> +		/*
> +		 * aer_info is a fixed 96-byte buffer, smaller than struct
> +		 * aer_capability_regs, so bound the copy to the source. Clear
> +		 * the software-only header_len and flit fields afterwards so
> +		 * firmware bytes 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
>   }


Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

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

* Re: [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections
  2026-07-17 16:16 ` [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections Dave Jiang
  2026-07-20 16:29   ` Alison Schofield
@ 2026-08-12 13:18   ` Shuai Xue
  1 sibling, 0 replies; 37+ messages in thread
From: Shuai Xue @ 2026-08-12 13:18 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, terry.bowman,
	sashiko-bot



On 7/18/26 12:16 AM, Dave Jiang wrote:
> sashiko-bot flagged a missing bounds check on data_length that allows
> an out-of-bounds read of elog_buf.
> 
> 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 checks that data_length stays
> within the buffer, so a malformed record can walk the section pointer
> past elog_buf and read adjacent memory.
> 
> Unlike the GHES paths, extlog never calls cper_estatus_check(). Reject a
> record whose length exceeds ELOG_ENTRY_LEN and run cper_estatus_check()
> before walking the sections.
> 
> A malformed record is now dropped with NOTIFY_DONE without setting
> MCE_HANDLED_EXTLOG, reflecting that 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")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v3:
> - New patch. sashiko's review of v2 pointed out extlog walks the elog
>    sections without the cper_estatus_check() bound the GHES paths use.
> ---
>   drivers/acpi/acpi_extlog.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 1fbaf134c04e..30863fd3f5b0 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -200,6 +200,13 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
>   
>   	tmp = (struct acpi_hest_generic_status *)elog_buf;
>   
> +	/*
> +	 * data_length is firmware controlled, so make sure the record and its
> +	 * sections stay within elog_buf before anything walks them.
> +	 */
> +	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;

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Thanks.
Shuai

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

* Re: [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko
  2026-07-21 15:06   ` Dave Jiang
@ 2026-08-14 20:16     ` Rafael J. Wysocki (Intel)
  0 siblings, 0 replies; 37+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-08-14 20:16 UTC (permalink / raw)
  To: Dave Jiang
  Cc: Rafael J. Wysocki (Intel), linux-acpi, linux-cxl, tony.luck, bp,
	guohanjun, mchehab, xueshuai, terry.bowman, Ben Cheatham

On Tue, Jul 21, 2026 at 5:06 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
>
>
> On 7/21/26 4:33 AM, Rafael J. Wysocki (Intel) wrote:
> > On Fri, Jul 17, 2026 at 6:16 PM Dave Jiang <dave.jiang@intel.com> wrote:
> >>
> >> A collection of fixes for pre-existing issues reported by sashiko-bot while
> >> reviewing patches.
> >>
> >> The v1 posting fixed a first batch of these issues, and v2 grew the set as
> >> sashiko-bot's review surfaced more problems in the same CPER/extlog paths.
> >> sashiko-bot's review of v2 flagged yet more of the same class of bug in
> >> neighbouring code, so v3 adds those as well: an out-of-range section length
> >> that defeats cper_estatus_check(), the same unvalidated AER buffer handling
> >> in ghes_handle_aer() that was already fixed for extlog, and an unbounded
> >> walk of the extlog record.
> >>
> >> 1/10:  Bound the CXL event record copy to the firmware section length.
> >> 2/10:  Reject CPER records with an out-of-range error_data_length.
> >> 3/10:  Validate the CXL protocol error section length before the RAS cap copy.
> >> 4/10:  Avoid populating software AER metadata from the raw hardware buffer.
> >> 5/10:  Validate the PCIe error section length before payload access.
> >> 6/10:  Fix the CONFIG_ACPI_APEI_PCIEAER guard typo in extlog.c.
> >> 7/10:  Defer CXL protocol error handling to avoid a lock inversion.
> >> 8/10:  Validate the memory error section length before payload access.
> >> 9/10:  Bound the AER info copy and sanitize software metadata in ghes.c.
> >> 10/10: Validate the extlog record length before walking sections.
> >>
> >> Note patch 2 touches drivers/firmware/efi/cper.c; it closes the length
> >> validation hole at the shared choke point that the per-section guards in the
> >> rest of the series rely on.
> >>
> >> sashiko-bot's review of v2 also flagged a few related issues that are not
> >> addressed here because they live in other subsystems.
> >>
> >>   - cxl_cper_print_prot_err() in drivers/firmware/efi/cper_cxl.c uses the
> >>     firmware-controlled 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 into a struct aer_capability_regs without clearing
> >>     the software-only header_len/flit fields, the same class of issue as
> >>     patches 4 and 9.
> >>
> >> 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/
> >>
> >> Changes since v2
> >> ----------------
> >> - Dropped the standalone spin_lock_irqsave() change; it is 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 bypass the check and 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 the CXL protocol error
> >>   handling is deferred to the workqueue, before the typo fix that activates
> >>   that code.
> >>
> >> Changes since v1
> >> ----------------
> >> - See the v2 posting for the full v1 -> v2 changelog.
> >>
> >>
> >> Dave Jiang (10):
> >>   ACPI: APEI: GHES: Bound CXL event record copy to the firmware section
> >>     length
> >>   efi/cper: Reject CPER records with an out-of-range error_data_length
> >>   ACPI: APEI: GHES: Validate CXL protocol error section length before
> >>     RAS cap copy
> >>   ACPI: extlog: Avoid populating software AER metadata from raw hardware
> >>     buffer
> >>   ACPI: extlog: Validate PCIe error section length before payload access
> >>   ACPI: extlog: Defer CXL protocol error handling to avoid lock
> >>     inversion
> >>   ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
> >>   ACPI: APEI: GHES: Validate memory error section length before payload
> >>     access
> >>   ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
> >>   ACPI: extlog: Validate elog record length before walking sections
> >>
> >>  drivers/acpi/acpi_extlog.c       | 47 ++++++++++++-------------
> >>  drivers/acpi/apei/ghes.c         | 59 +++++++++++++++++++++++++-------
> >>  drivers/acpi/apei/ghes_helpers.c | 21 +++++++++++-
> >>  drivers/firmware/efi/cper.c      | 10 ++++++
> >>  include/acpi/ghes.h              |  4 +++
> >>  include/cxl/event.h              |  4 +--
> >>  6 files changed, 106 insertions(+), 39 deletions(-)
> >
> > All of this looks reasonable to me, but it really is material for Tony
> > to look at.
>
> Thanks Rafael. I've asked Tony to take a look. Looks like he's on vacation so hopefully soon.

After Shuai Xue's review this is good to go from my perspective, but
please address the review feedback.

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

end of thread, other threads:[~2026-08-14 20:16 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17 16:16 [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
2026-07-17 16:16 ` [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
2026-07-20 12:21   ` Hanjun Guo
2026-07-20 16:15     ` Dave Jiang
2026-07-20 16:23   ` Alison Schofield
2026-08-12 12:10   ` Shuai Xue
2026-07-17 16:16 ` [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length Dave Jiang
2026-07-20 16:24   ` Alison Schofield
2026-08-12 12:23   ` Shuai Xue
2026-07-17 16:16 ` [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
2026-07-20 16:25   ` Alison Schofield
2026-08-12 12:30   ` Shuai Xue
2026-07-17 16:16 ` [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer Dave Jiang
2026-07-20 16:25   ` Alison Schofield
2026-08-12 12:32   ` Shuai Xue
2026-07-17 16:16 ` [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access Dave Jiang
2026-07-20 16:26   ` Alison Schofield
2026-08-12 12:59   ` Shuai Xue
2026-07-17 16:16 ` [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion Dave Jiang
2026-07-20 16:26   ` Alison Schofield
2026-08-12 13:06   ` Shuai Xue
2026-07-17 16:16 ` [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
2026-07-17 21:24   ` Dave Jiang
2026-07-20 16:27   ` Alison Schofield
2026-08-12 13:10   ` Shuai Xue
2026-07-17 16:16 ` [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access Dave Jiang
2026-07-20 16:28   ` Alison Schofield
2026-08-12 13:14   ` Shuai Xue
2026-07-17 16:16 ` [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata Dave Jiang
2026-07-20 16:28   ` Alison Schofield
2026-08-12 13:16   ` Shuai Xue
2026-07-17 16:16 ` [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections Dave Jiang
2026-07-20 16:29   ` Alison Schofield
2026-08-12 13:18   ` Shuai Xue
2026-07-21 11:33 ` [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Rafael J. Wysocki (Intel)
2026-07-21 15:06   ` Dave Jiang
2026-08-14 20:16     ` Rafael J. Wysocki (Intel)

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