Linux CXL
 help / color / mirror / Atom feed
* [PATCH 0/4] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko
@ 2026-07-09 16:28 Dave Jiang
  2026-07-09 16:28 ` [PATCH 1/4] ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks Dave Jiang
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Dave Jiang @ 2026-07-09 16:28 UTC (permalink / raw)
  To: linux-acpi, linux-cxl; +Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai

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

1/4: Use spin_lock_irqsave() instead of spin_lock() for register/unregister
     of CXL CPER work.
2/4: Bound CXL check the firmware section length before copy.
3/4: Fix CONFIG_ACPI_APEI_PCIEAER guard typo in extlog.c.
4/4: Validate CXL protocol error section length before RAS capability copy


Dave Jiang (4):
  ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks
  ACPI: APEI: GHES: Bound CXL event record copy to the firmware section
    length
  ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
  ACPI: APEI: GHES: Validate CXL protocol error section length before
    RAS cap copy

 drivers/acpi/acpi_extlog.c       | 11 +++++-----
 drivers/acpi/apei/ghes.c         | 37 ++++++++++++++++++++++----------
 drivers/acpi/apei/ghes_helpers.c | 15 ++++++++++++-
 include/cxl/event.h              |  4 ++--
 4 files changed, 48 insertions(+), 19 deletions(-)


base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
-- 
2.54.0


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

* [PATCH 1/4] ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks
  2026-07-09 16:28 [PATCH 0/4] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
@ 2026-07-09 16:28 ` Dave Jiang
  2026-07-09 16:46   ` sashiko-bot
  2026-07-10 17:27   ` Cheatham, Benjamin
  2026-07-09 16:28 ` [PATCH 2/4] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Dave Jiang @ 2026-07-09 16:28 UTC (permalink / raw)
  To: linux-acpi, linux-cxl; +Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai

cxl_cper_post_event() and cxl_cper_post_prot_err() acquire
cxl_cper_work_lock and cxl_cper_prot_err_work_lock with
spin_lock_irqsave(). These producers run from ghes_do_proc() in hard IRQ
context (the GHES IRQ handler) and, for SEA/NMI notifications, from NMI
context.

The register/unregister helpers take the same two locks with a plain
spin_lock() from process context (cxl_ras_init()/cxl_ras_exit() at module
load and unload). If a GHES error interrupt is delivered to a CPU that is
holding one of these locks via the plain spin_lock(), the interrupt
handler's spin_lock_irqsave() on the same lock spins forever against a
lock the interrupted thread can no longer release.

Take these locks with spin_lock_irqsave() on the register/unregister
paths as well.

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")
Fixes: 36f257e3b0ba ("acpi/ghes, cxl/pci: Process CXL CPER Protocol Errors")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/apei/ghes.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 3236a3ce79d6..9b6917698ed2 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -783,7 +783,7 @@ int cxl_cper_register_prot_err_work(struct work_struct *work)
 	if (cxl_cper_prot_err_work)
 		return -EINVAL;
 
-	guard(spinlock)(&cxl_cper_prot_err_work_lock);
+	guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
 	cxl_cper_prot_err_work = work;
 	return 0;
 }
@@ -794,7 +794,7 @@ int cxl_cper_unregister_prot_err_work(struct work_struct *work)
 	if (cxl_cper_prot_err_work != work)
 		return -EINVAL;
 
-	guard(spinlock)(&cxl_cper_prot_err_work_lock);
+	guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
 	cxl_cper_prot_err_work = NULL;
 	return 0;
 }
@@ -852,7 +852,7 @@ int cxl_cper_register_work(struct work_struct *work)
 	if (cxl_cper_work)
 		return -EINVAL;
 
-	guard(spinlock)(&cxl_cper_work_lock);
+	guard(spinlock_irqsave)(&cxl_cper_work_lock);
 	cxl_cper_work = work;
 	return 0;
 }
@@ -863,7 +863,7 @@ int cxl_cper_unregister_work(struct work_struct *work)
 	if (cxl_cper_work != work)
 		return -EINVAL;
 
-	guard(spinlock)(&cxl_cper_work_lock);
+	guard(spinlock_irqsave)(&cxl_cper_work_lock);
 	cxl_cper_work = NULL;
 	return 0;
 }
-- 
2.54.0


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

* [PATCH 2/4] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length
  2026-07-09 16:28 [PATCH 0/4] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
  2026-07-09 16:28 ` [PATCH 1/4] ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks Dave Jiang
@ 2026-07-09 16:28 ` Dave Jiang
  2026-07-09 16:42   ` sashiko-bot
  2026-07-09 16:28 ` [PATCH 3/4] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
  2026-07-09 16:28 ` [PATCH 4/4] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
  3 siblings, 1 reply; 11+ messages in thread
From: Dave Jiang @ 2026-07-09 16:28 UTC (permalink / raw)
  To: linux-acpi, linux-cxl; +Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai

cxl_cper_post_event() blindly copies a fixed
sizeof(struct cxl_cper_event_rec) out of the firmware-provided CPER
section without checking the length. Pass gdata->err_data_length into
the function and reject a section too small to hold the record before
the copy.

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 | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 9b6917698ed2..7b465e2afcb0 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -815,10 +815,21 @@ 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;
 
+	/*
+	 * The record is copied whole below, so the firmware-provided section
+	 * must be at least as large as the record. cper_estatus_check() only
+	 * validates the section fits the error block, not that it satisfies a
+	 * given section type's size, so guard the fixed-size copy here.
+	 */
+	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 +960,18 @@ static void ghes_do_proc(struct ghes *ghes,
 		} else if (guid_equal(sec_type, &CPER_SEC_CXL_GEN_MEDIA_GUID)) {
 			struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
 
-			cxl_cper_post_event(CXL_CPER_EVENT_GEN_MEDIA, rec);
+			cxl_cper_post_event(CXL_CPER_EVENT_GEN_MEDIA, rec,
+					    gdata->error_data_length);
 		} else if (guid_equal(sec_type, &CPER_SEC_CXL_DRAM_GUID)) {
 			struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
 
-			cxl_cper_post_event(CXL_CPER_EVENT_DRAM, rec);
+			cxl_cper_post_event(CXL_CPER_EVENT_DRAM, rec,
+					    gdata->error_data_length);
 		} else if (guid_equal(sec_type, &CPER_SEC_CXL_MEM_MODULE_GUID)) {
 			struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
 
-			cxl_cper_post_event(CXL_CPER_EVENT_MEM_MODULE, rec);
+			cxl_cper_post_event(CXL_CPER_EVENT_MEM_MODULE, rec,
+					    gdata->error_data_length);
 		} else {
 			void *err = acpi_hest_get_payload(gdata);
 
-- 
2.54.0


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

* [PATCH 3/4] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
  2026-07-09 16:28 [PATCH 0/4] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
  2026-07-09 16:28 ` [PATCH 1/4] ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks Dave Jiang
  2026-07-09 16:28 ` [PATCH 2/4] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
@ 2026-07-09 16:28 ` Dave Jiang
  2026-07-09 16:54   ` sashiko-bot
  2026-07-10 17:27   ` Cheatham, Benjamin
  2026-07-09 16:28 ` [PATCH 4/4] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
  3 siblings, 2 replies; 11+ messages in thread
From: Dave Jiang @ 2026-07-09 16:28 UTC (permalink / raw)
  To: linux-acpi, linux-cxl; +Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai

"#ifdef ACPI_APEI_PCIEAER" is incorrect. It should be
"#ifdef CONFIG_ACPI_APEI_PCIEAE". Currently the section wrapped will
always evalutes to the stub because of this. Use the correct CONFIG_
prefixed symbol so the code is actually built.

Fixes: 95350effc3ad ("ACPI: extlog: Trace CPER CXL Protocol Error Section")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/acpi_extlog.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 7ad3b36013cc..60c2b90adf0a 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)
 {
-#ifdef ACPI_APEI_PCIEAER
+#ifdef CONFIG_ACPI_APEI_PCIEAER
 	struct aer_capability_regs *aer;
 	struct pci_dev *pdev;
 	unsigned int devfn;
@@ -167,7 +167,7 @@ static void
 extlog_cxl_cper_handle_prot_err(struct cxl_cper_sec_prot_err *prot_err,
 				int severity)
 {
-#ifdef ACPI_APEI_PCIEAER
+#ifdef CONFIG_ACPI_APEI_PCIEAER
 	struct cxl_cper_prot_err_work_data wd;
 
 	if (cxl_cper_sec_prot_err_valid(prot_err))
-- 
2.54.0


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

* [PATCH 4/4] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy
  2026-07-09 16:28 [PATCH 0/4] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
                   ` (2 preceding siblings ...)
  2026-07-09 16:28 ` [PATCH 3/4] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
@ 2026-07-09 16:28 ` Dave Jiang
  2026-07-10 17:27   ` Cheatham, Benjamin
  3 siblings, 1 reply; 11+ messages in thread
From: Dave Jiang @ 2026-07-09 16:28 UTC (permalink / raw)
  To: linux-acpi, linux-cxl; +Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai

cxl_cper_setup_prot_err_work_data() locates the RAS Capability block by
skipping a firmware-controlled DVSEC:

	dvsec_start = (u8 *)(prot_err + 1);
	cap_start = dvsec_start + prot_err->dvsec_len;
	memcpy(&wd->ras_cap, cap_start, sizeof(wd->ras_cap));

prot_err->dvsec_len is taken from the CPER section and is never validated.

Add the check to cxl_cper_sec_prot_err_valid() to enforce the check and
avoid copying out of bounds.

Link: https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=6
Fixes: 315c2f0b53ba ("acpi/ghes, cper: Recognize and cache CXL Protocol errors")
Assisted-by: Claude:claude-opus-4-8
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 | 15 ++++++++++++++-
 include/cxl/event.h              |  4 ++--
 4 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
index 60c2b90adf0a..c65b54740bf5 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 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;
 
 	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 7b465e2afcb0..5af794fed242 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);
@@ -956,7 +956,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..8422f6daff86 100644
--- a/drivers/acpi/apei/ghes_helpers.c
+++ b/drivers/acpi/apei/ghes_helpers.c
@@ -5,7 +5,7 @@
 #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 (!(prot_err->valid_bits & PROT_ERR_VALID_AGENT_ADDRESS)) {
 		pr_err_ratelimited("CXL CPER invalid agent type\n");
@@ -23,6 +23,19 @@ 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
+	 * prot_err->dvsec_len bytes. Verify the header, the DVSEC and the RAS
+	 * Capability block all fit within the CPER 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.54.0


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

* Re: [PATCH 2/4] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length
  2026-07-09 16:28 ` [PATCH 2/4] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
@ 2026-07-09 16:42   ` sashiko-bot
  0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-09 16:42 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linux-cxl

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for out-of-bounds read vulnerability when parsing CXL Protocol Error and other CPER sections.
--

--- Patch [2]: [PATCH 2/4] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length ---
commit 4eb9f7aafa36f7ab79ba6d995ac9206e430ee1b0
Author: Dave Jiang <dave.jiang@intel.com>

ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

This commit addresses an issue where cxl_cper_post_event() copies a fixed-size
record from the firmware-provided CPER section without validating the section
length. It passes error_data_length to the function and rejects sections that
are too small.

Link: https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=6

> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 9b6917698ed24..7b465e2afcb04 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c

[ ... ]

> @@ -949,15 +960,18 @@ static void ghes_do_proc(struct ghes *ghes,

[Severity: High]
This isn't a bug introduced by this patch, but while bounds checking is being
added for CXL media and DRAM events here, the same vulnerability appears to
remain for CXL Protocol Error and other CPER sections nearby.

Does the code pass the payload to cxl_cper_post_prot_err() without
validating if gdata->error_data_length is sufficient for the struct?

This could lead to an out-of-bounds memory read in
cxl_cper_setup_prot_err_work_data() when copying the structs:

drivers/acpi/apei/ghes_helpers.c:cxl_cper_setup_prot_err_work_data() {
    ...
    cap_start = dvsec_start + prot_err->dvsec_len;
    memcpy(&wd->ras_cap, cap_start, sizeof(wd->ras_cap));
    ...
}

If the firmware provides a CPER_SEC_CXL_PROT_ERR section smaller than
expected, could dvsec_len be read from uninitialized memory and used as an
offset, potentially triggering a page fault?

Similar missing length validation also appears to be present when calling
extlog_cxl_cper_handle_prot_err() in drivers/acpi/acpi_extlog.c:extlog_print(),
and when accessing mem_err payloads in ghes_handle_memory_failure():

drivers/acpi/apei/ghes.c:ghes_handle_memory_failure() {
    ...
    if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
    ...
}

>  		} 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);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709162807.1957783-1-dave.jiang@intel.com?part=2

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

* Re: [PATCH 1/4] ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks
  2026-07-09 16:28 ` [PATCH 1/4] ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks Dave Jiang
@ 2026-07-09 16:46   ` sashiko-bot
  2026-07-10 17:27   ` Cheatham, Benjamin
  1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-09 16:46 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linux-cxl

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds check on CPER record length and `dvsec_len` allows an out-of-bounds memory read that can be leaked to userspace via trace events.
--

--- Patch [1]: [PATCH 1/4] ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks ---
commit 92925c77a9085c947d2733f2a851c54ec53288f8
Author: Dave Jiang <dave.jiang@intel.com>

ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks

This patch switches the cxl_cper_work_lock and cxl_cper_prot_err_work_lock
from plain spinlocks to spin_lock_irqsave(). This prevents a potential deadlock
when a GHES error interrupt interrupts a thread holding the lock in process
context.

Link: https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=6

> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 3236a3ce79d6b..9b6917698ed24 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
[ ... ]
> @@ -863,7 +863,7 @@ int cxl_cper_unregister_work(struct work_struct *work)
>  	if (cxl_cper_work != work)
>  		return -EINVAL;
>  
> -	guard(spinlock)(&cxl_cper_work_lock);
> +	guard(spinlock_irqsave)(&cxl_cper_work_lock);
>  	cxl_cper_work = NULL;
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but looking at the CXL CPER work queue
handling, is there a potential out-of-bounds read when processing
malformed CPER records?

In drivers/acpi/apei/ghes_helpers.c:cxl_cper_setup_prot_err_work_data(),
the cap_start pointer is computed using an untrusted dvsec_len from the
firmware:

    dvsec_start = (u8 *)(prot_err + 1);
    cap_start = dvsec_start + prot_err->dvsec_len;

    memcpy(&wd->ras_cap, cap_start, sizeof(wd->ras_cap));

Since dvsec_len is provided by firmware, could a maliciously large value
cause cap_start to point outside the mapped CPER record bounds?

Similarly, in drivers/acpi/apei/ghes.c:cxl_cper_post_event(), the code
copies the event record:

    memcpy(&wd.rec, rec, sizeof(wd.rec));

If a hardware device provides a malformed record with a small
error_data_length or an unusually large dvsec_len, could this unconditional
memcpy() read unmapped memory or leak kernel memory contents to userspace
via the kfifo trace events?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709162807.1957783-1-dave.jiang@intel.com?part=1

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

* Re: [PATCH 3/4] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
  2026-07-09 16:28 ` [PATCH 3/4] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
@ 2026-07-09 16:54   ` sashiko-bot
  2026-07-10 17:27   ` Cheatham, Benjamin
  1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-07-09 16:54 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linux-cxl

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds checking on firmware-provided CPER data lengths before accessing or copying PCIe and CXL protocol error structures.
- [High] Calling `cxl_cper_handle_prot_err` synchronously creates an AB-BA lock ordering deadlock with PCI `device_lock` and `x86_mce_decoder_chain.rwsem`.
- [High] Casting the raw hardware `aer_info` buffer directly to `struct aer_capability_regs` populates software-only metadata with unvalidated hardware data, triggering out-of-bounds array reads.
- [Low] Directly calling `cxl_cper_handle_prot_err()` causes a linkage failure when `CONFIG_ACPI_EXTLOG=y` and `CONFIG_CXL_BUS` is configured as a module.
--

--- Patch [3]: [PATCH 3/4] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo ---
commit d29f05a05af2b3add29bfc216f34a5de8a53eb2a
Author: Dave Jiang <dave.jiang@intel.com>

ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

This patch fixes a typo in the preprocessor guard for ACPI_APEI_PCIEAER.
It changes the guard to use the correct CONFIG_ prefix, ensuring the
enclosed PCIe AER and CXL CPER handling code is actually built when enabled.

> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 7ad3b36013cc6..60c2b90adf0ae 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)
>  {
> -#ifdef ACPI_APEI_PCIEAER
> +#ifdef CONFIG_ACPI_APEI_PCIEAER

[Severity: High]
This isn't a bug introduced by this patch, but activating this code exposes
a missing bounds check on firmware-provided CPER data lengths.

If the firmware provides a CPER record where gdata->error_data_length is
smaller than the expected payload structs, or provides a maliciously large
dvsec_len for a CXL Protocol Error, is there a risk of an out-of-bounds
memory read?

When extlog_print() extracts payloads and dispatches to handlers without
verifying gdata->error_data_length >= sizeof(...), the code casts the
payload and executes cxl_cper_setup_prot_err_work_data(). This function
performs pointer arithmetic using the unvalidated dvsec_len and executes
a fixed-size memcpy(), which could lead to a kernel panic or data leak.

[Severity: High]
This is a pre-existing issue, but fixing the typo activates a code path in
extlog_print_pcie() that can trigger out-of-bounds array reads.

Does casting the raw 96-byte aer_info hardware buffer directly to struct
aer_capability_regs * populate software-only metadata with unvalidated
hardware data?

In Linux, struct aer_capability_regs embeds struct pcie_tlp_log, placing
software metadata (header_len and flit) at offset 0x54. Casting raw
registers populates these software fields with unvalidated hardware state.

If flit evaluates to true and header_len is read as a large value (up to
255), pcie_print_tlp_log() will iterate up to 255 times, reading
out-of-bounds past the 14-element log->dw array. This read traverses past
the aer_info struct and can cross into unmapped memory.

>  	struct aer_capability_regs *aer;
>  	struct pci_dev *pdev;

[ ... ]

> @@ -167,7 +167,7 @@ static void
>  extlog_cxl_cper_handle_prot_err(struct cxl_cper_sec_prot_err *prot_err,
>  				int severity)
>  {
> -#ifdef ACPI_APEI_PCIEAER
> +#ifdef CONFIG_ACPI_APEI_PCIEAER

[Severity: Low]
This is a pre-existing issue, but activating this previously dead code causes
a linkage failure when CONFIG_ACPI_EXTLOG=y and CONFIG_CXL_BUS is configured
as a module.

Because ACPI_EXTLOG does not depend on CXL_BUS in its Kconfig, is it possible
that the direct symbol call to cxl_cper_handle_prot_err() from the built-in
acpi_extlog.o to the module cxl_core.ko cannot be resolved? This would result
in an undefined reference to cxl_cper_handle_prot_err.

[Severity: High]
This isn't a bug introduced by this patch, but activating this code exposes a
potential AB-BA lock ordering deadlock with the PCI device_lock and
x86_mce_decoder_chain.rwsem.

When extlog_cxl_cper_handle_prot_err() is called synchronously while the
notifier rwsem is held, does this create a circular dependency?

extlog_print() is registered as an MCE decoder and is called with
down_read(&x86_mce_decoder_chain.rwsem) held. It now synchronously calls
cxl_cper_handle_prot_err(), which acquires the PCI device lock via
guard(device)(&pdev->dev), establishing the lock order:
x86_mce_decoder_chain.rwsem -> device_lock.

Conversely, during system initialization, drivers (like AMD GPU or CXL via
devm_cxl_register_mce_notifier()) execute .probe with the PCI device_lock
held and call mce_register_decode_chain(), taking
down_write(&x86_mce_decoder_chain.rwsem). Could this cause a hard deadlock if
an MCE occurs during driver probing or initialization?

>  	struct cxl_cper_prot_err_work_data wd;
>  
>  	if (cxl_cper_sec_prot_err_valid(prot_err))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709162807.1957783-1-dave.jiang@intel.com?part=3

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

* Re: [PATCH 1/4] ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks
  2026-07-09 16:28 ` [PATCH 1/4] ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks Dave Jiang
  2026-07-09 16:46   ` sashiko-bot
@ 2026-07-10 17:27   ` Cheatham, Benjamin
  1 sibling, 0 replies; 11+ messages in thread
From: Cheatham, Benjamin @ 2026-07-10 17:27 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai

On 7/9/2026 11:28 AM, Dave Jiang wrote:
> cxl_cper_post_event() and cxl_cper_post_prot_err() acquire
> cxl_cper_work_lock and cxl_cper_prot_err_work_lock with
> spin_lock_irqsave(). These producers run from ghes_do_proc() in hard IRQ
> context (the GHES IRQ handler) and, for SEA/NMI notifications, from NMI
> context.
> 
> The register/unregister helpers take the same two locks with a plain
> spin_lock() from process context (cxl_ras_init()/cxl_ras_exit() at module
> load and unload). If a GHES error interrupt is delivered to a CPU that is
> holding one of these locks via the plain spin_lock(), the interrupt
> handler's spin_lock_irqsave() on the same lock spins forever against a
> lock the interrupted thread can no longer release.
> 
> Take these locks with spin_lock_irqsave() on the register/unregister
> paths as well.
> 
> 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")
> Fixes: 36f257e3b0ba ("acpi/ghes, cxl/pci: Process CXL CPER Protocol Errors")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---

Since the Sashiko review is for a different issue and this makes sense to me:
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

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

* Re: [PATCH 3/4] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
  2026-07-09 16:28 ` [PATCH 3/4] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
  2026-07-09 16:54   ` sashiko-bot
@ 2026-07-10 17:27   ` Cheatham, Benjamin
  1 sibling, 0 replies; 11+ messages in thread
From: Cheatham, Benjamin @ 2026-07-10 17:27 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai

On 7/9/2026 11:28 AM, Dave Jiang wrote:
> "#ifdef ACPI_APEI_PCIEAER" is incorrect. It should be
> "#ifdef CONFIG_ACPI_APEI_PCIEAE". Currently the section wrapped will
> always evalutes to the stub because of this. Use the correct CONFIG_
> prefixed symbol so the code is actually built.
> 
> Fixes: 95350effc3ad ("ACPI: extlog: Trace CPER CXL Protocol Error Section")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/acpi/acpi_extlog.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 7ad3b36013cc..60c2b90adf0a 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)
>  {
> -#ifdef ACPI_APEI_PCIEAER
> +#ifdef CONFIG_ACPI_APEI_PCIEAER
>  	struct aer_capability_regs *aer;
>  	struct pci_dev *pdev;
>  	unsigned int devfn;
> @@ -167,7 +167,7 @@ static void
>  extlog_cxl_cper_handle_prot_err(struct cxl_cper_sec_prot_err *prot_err,
>  				int severity)
>  {
> -#ifdef ACPI_APEI_PCIEAER
> +#ifdef CONFIG_ACPI_APEI_PCIEAER

Seems like Sashiko is complaining about bugs that it already highlighted in patch 2/4 (I think?). But it did point
out a linker error here that needs to be fixed. Should be able to fix it by adding CONFIG_CXL_BUS to the #ifdef
or a new nested #ifdef CONFIG_CXL_BUS. Either works for me, so with that applied:

Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

>  	struct cxl_cper_prot_err_work_data wd;
>  
>  	if (cxl_cper_sec_prot_err_valid(prot_err))


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

* Re: [PATCH 4/4] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy
  2026-07-09 16:28 ` [PATCH 4/4] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
@ 2026-07-10 17:27   ` Cheatham, Benjamin
  0 siblings, 0 replies; 11+ messages in thread
From: Cheatham, Benjamin @ 2026-07-10 17:27 UTC (permalink / raw)
  To: Dave Jiang, linux-acpi, linux-cxl
  Cc: rafael, tony.luck, bp, guohanjun, mchehab, xueshuai

On 7/9/2026 11:28 AM, Dave Jiang wrote:
> cxl_cper_setup_prot_err_work_data() locates the RAS Capability block by
> skipping a firmware-controlled DVSEC:
> 
> 	dvsec_start = (u8 *)(prot_err + 1);
> 	cap_start = dvsec_start + prot_err->dvsec_len;
> 	memcpy(&wd->ras_cap, cap_start, sizeof(wd->ras_cap));
> 
> prot_err->dvsec_len is taken from the CPER section and is never validated.
> 
> Add the check to cxl_cper_sec_prot_err_valid() to enforce the check and
> avoid copying out of bounds.
> 
> Link: https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=6
> Fixes: 315c2f0b53ba ("acpi/ghes, cper: Recognize and cache CXL Protocol errors")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---

You may want to check the compilation issue with the last patch isn't also present here. I think I've convinced myself it isn't,
but it can't hurt. If it's fine:

Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

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

end of thread, other threads:[~2026-07-10 17:27 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 16:28 [PATCH 0/4] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko Dave Jiang
2026-07-09 16:28 ` [PATCH 1/4] ACPI: APEI: GHES: Use spin_lock_irqsave() for CXL CPER work locks Dave Jiang
2026-07-09 16:46   ` sashiko-bot
2026-07-10 17:27   ` Cheatham, Benjamin
2026-07-09 16:28 ` [PATCH 2/4] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length Dave Jiang
2026-07-09 16:42   ` sashiko-bot
2026-07-09 16:28 ` [PATCH 3/4] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
2026-07-09 16:54   ` sashiko-bot
2026-07-10 17:27   ` Cheatham, Benjamin
2026-07-09 16:28 ` [PATCH 4/4] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
2026-07-10 17:27   ` Cheatham, Benjamin

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