* [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
` (9 more replies)
0 siblings, 10 replies; 11+ 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] 11+ 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-17 16:16 ` [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length Dave Jiang
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ 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] 11+ 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-17 16:16 ` [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ 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] 11+ 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-17 16:16 ` [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer Dave Jiang
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ 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] 11+ 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-17 16:16 ` [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access Dave Jiang
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ 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] 11+ 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-17 16:16 ` [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion Dave Jiang
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ 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] 11+ 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-17 16:16 ` [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ 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] 11+ 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 16:16 ` [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access Dave Jiang
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ 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] 11+ 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-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 ` [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections Dave Jiang
9 siblings, 0 replies; 11+ 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] 11+ 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-17 16:16 ` [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections Dave Jiang
9 siblings, 0 replies; 11+ 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] 11+ 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
9 siblings, 0 replies; 11+ 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] 11+ messages in thread
end of thread, other threads:[~2026-07-17 16:17 UTC | newest]
Thread overview: 11+ 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-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 ` [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy Dave Jiang
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 ` [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access Dave Jiang
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 ` [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo Dave Jiang
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 ` [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata Dave Jiang
2026-07-17 16:16 ` [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections Dave Jiang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox