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

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

[Dave Jiang]

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

[PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

[Dave Jiang]

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

Re: [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fixes an out-of-bounds read in cxl_cper_post_event(), but fails to apply a similar bounds check to the adjacent CXL Protocol Error handler for CPER_SEC_CXL_PROT_ERR.
- [High] Missing bounds checks for CPER_SEC_CXL_PROT_ERR and CPER_SEC_PCIE in the ACPI Extended Error Log (acpi_extlog.c), leading to out-of-bounds reads.
- [High] Missing bounds check for HISI PCIe vendor-specific error records, causing an out-of-bounds read.
- [High] Spinlock deadlock in CXL CPER work queue registration functions due to failure to disable interrupts.
--

--- Patch [1]: [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length ---
commit 36a126e36333bbe9036612ec43459f24c942a9bb
Author: Dave Jiang <dave.jiang@intel.com>
Subject: ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

This commit addresses an out-of-bounds read of the firmware CPER section by
adding bounds checking for CXL event records. It updates cxl_cper_post_event()
to verify that the payload length matches expected structure sizes.

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..a752e152a5a0b 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -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);
>  

[Severity: High]
This is a pre-existing issue, but does CPER_SEC_CXL_PROT_ERR also need a
similar bounds check in ghes_do_proc()?

The payload is passed to cxl_cper_post_prot_err() without validating the outer
section length:

drivers/acpi/apei/ghes.c:ghes_do_proc()
  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);
  }

Then in cxl_cper_setup_prot_err_work_data(), an out-of-bounds copy can occur
based on an unvalidated dvsec_len:

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

Could an untrusted firmware payload lead to an out-of-bounds read here?

[Severity: High]
This is a pre-existing issue, but are there missing bounds checks in other
consumers of these error records, like drivers/acpi/acpi_extlog.c?

In extlog_print(), the payload for CXL protocol errors is passed without
checking if gdata->error_data_length is large enough:

drivers/acpi/acpi_extlog.c:extlog_print()
  extlog_cxl_cper_handle_prot_err(prot_err, gdata->error_severity);

Similarly, in extlog_print_pcie(), the payload is dereferenced directly:

drivers/acpi/acpi_extlog.c:extlog_print_pcie()
  if (!(pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID && ...))

Could this result in out-of-bounds reads if the firmware provides truncated
payload sections?

[Severity: High]
This is a pre-existing issue, but does hisi_pcie_notify_error() in
drivers/pci/controller/pcie-hisi-error.c also lack bounds checking for
vendor-specific error records?

The payload from gdata is directly cast and dereferenced without ensuring
gdata->error_data_length covers the structure:

drivers/pci/controller/pcie-hisi-error.c:hisi_pcie_notify_error()
  if (error_data->socket_id != socket)

Could a malformed firmware error record cause an out-of-bounds read here?

[Severity: High]
This is a pre-existing issue, but is there a potential spinlock deadlock in
the CXL CPER work queue registration functions?

In cxl_cper_register_work(), the lock is acquired without disabling interrupts:

drivers/acpi/apei/ghes.c:cxl_cper_register_work()
  guard(spinlock)(&cxl_cper_work_lock);

If a hardware error interrupt fires on the same CPU while the lock is held,
the interrupt handler calls cxl_cper_post_event(), which attempts to acquire
the same lock:

drivers/acpi/apei/ghes.c:cxl_cper_post_event()
  guard(spinlock_irqsave)(&cxl_cper_work_lock);

Could this cause a deadlock if the interrupt occurs when the driver is
registering the work queue?

A similar issue appears in cxl_cper_register_prot_err_work() with
cxl_cper_prot_err_work_lock.

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

Re: [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

[Hanjun Guo]

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

Re: [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

[Dave Jiang]

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

Re: [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

[Alison Schofield]

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>

Re: [PATCH v3 01/10] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

[Shuai Xue]

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

[PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length

[Dave Jiang]

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

Re: [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length

[sashiko-bot]

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

Pre-existing issues:
- [High] The `extlog_print` path iterates over CPER sections without validating the firmware data bounds or lengths, leaving it vulnerable to out-of-bounds reads and integer overflows.
- [High] A malformed CPER record with trailing bytes less than the size of a section header causes an out-of-bounds read in `cper_estatus_check()`.
--

--- Patch [2]: [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length ---
commit dc1e1608d652687fa8e464b2ae6a04fa5c4f8848
Author: Dave Jiang <dave.jiang@intel.com>

efi/cper: Reject CPER records with an out-of-range error_data_length

This commit fixes an integer overflow vulnerability in the shared CPER length
check. It ensures that error_data_length is not negative and does not exceed
the remaining data length before size arithmetic.

Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=1

> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index 06b4fdb59917a..99a86b2675e39 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;
>  

[Severity: High]
This is a pre-existing issue, but could the acpi_hest_get_size() call perform
an out-of-bounds read if data_len is very small?

If a malformed record leaves a small number of trailing bytes (for example,
data_len drops to 1), the iteration macro continues because the section
offset is still less than the total data_length.

The acpi_hest_get_size() helper calls acpi_hest_get_version(), which
dereferences gdata->revision at offset 20. Would this read memory past the
end of the validated CPER data region before we check if data_len is large
enough to contain the generic header itself?

[Severity: High]
This isn't a bug introduced by this patch, but is the extlog_print() path
bypassing these safety checks entirely?

Looking at extlog_print(), it iterates over CPER sections without validating
the firmware data bounds:

drivers/acpi/acpi_extlog.c:extlog_print() {
    ...
    tmp = (struct acpi_hest_generic_status *)elog_buf;
    ...
    apei_estatus_for_each_section(tmp, gdata) {
    ...
}

It copies up to ELOG_ENTRY_LEN (4096) bytes of estatus from firmware into
elog_buf, but doesn't appear to call cper_estatus_check() to validate it.

If firmware provides a data_length > 4096, will this loop read past the
bounds of elog_buf? Furthermore, does this leave the unvalidated path
vulnerable to the same error_data_length integer overflow issue being fixed
here?

> +		/*
> +		 * 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;

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

Re: [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length

[Alison Schofield]

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>

Re: [PATCH v3 02/10] efi/cper: Reject CPER records with an out-of-range error_data_length

[Shuai Xue]

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

[PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy

[Dave Jiang]

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

Re: [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing payload length validation for PCIe AER error records (`CPER_SEC_PCIE`).
- [High] The exact same out-of-bounds read vulnerability exists in `cxl_cper_print_prot_err()` when printing the CPER record.
- [High] Potential deadlock due to missing IRQ masking when acquiring `cxl_cper_work_lock` and `cxl_cper_prot_err_work_lock` in process context.
- [High] The `#ifdef ACPI_APEI_PCIEAER` guard in `drivers/acpi/acpi_extlog.c` is missing the `CONFIG_` prefix.
--

--- Patch [3]: [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy ---
commit d9ae66d42856680b3ac8a462d2465e800e436efc
Author: Dave Jiang <dave.jiang@intel.com>

ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy

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.

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/

> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 7ad3b36013cc6..06a944dadbc15 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

[Severity: High]
This isn't a bug introduced by this patch, but does this #ifdef evaluate to
false because it is missing the CONFIG_ prefix?

If so, does that cause the entire CXL protocol error handling and PCIe AER
logging in extlog_print_pcie() to be compiled as dead code?

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

[ ... ]

> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index a752e152a5a0b..17e4ef5552925 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);

[Severity: High]
This is a pre-existing issue, but while looking at how this lock is used,
I noticed a potential IRQ inversion deadlock.

In drivers/acpi/apei/ghes.c:cxl_cper_register_prot_err_work() and
cxl_cper_register_work(), the locks are acquired from process context using
guard(spinlock), which does not disable interrupts:

    guard(spinlock)(&cxl_cper_prot_err_work_lock);

If a hardware error occurs on the same CPU, the NMI/IRQ handler will call
cxl_cper_post_prot_err() which attempts to acquire the exact same spinlock
using guard(spinlock_irqsave):

    guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);

Could this cause a deadlock during initialization?

[ ... ]

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

[Severity: High]
This is a pre-existing issue, but I noticed similar validation might be
missing for PCIe AER error records.

When ghes_do_proc() calls ghes_handle_aer() for CPER_SEC_PCIE, it casts
gdata to struct cper_sec_pcie * and reads pcie_err->validation_bits without
first verifying that gdata->error_data_length is at least
sizeof(struct cper_sec_pcie).

Could malicious or buggy firmware supplying an undersized error data length
cause an out-of-bounds read here?

[ ... ]

> diff --git a/drivers/acpi/apei/ghes_helpers.c b/drivers/acpi/apei/ghes_helpers.c
> index bc7111b740afb..d625ec98a24c7 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)
>  {

[Severity: High]
This is a pre-existing issue, but I noticed another location that might need
similar bounds checking.

In drivers/firmware/efi/cper_cxl.c:cxl_cper_print_prot_err(), the function
uses the unvalidated prot_err->dvsec_len to calculate the offset to the RAS
capability registers:

    cxl_ras = (struct cxl_ras_capability_regs *)((long)prot_err + size);
    pr_info("%s cxl_ras_uncor_status: 0x%08x", pfx, cxl_ras->uncor_status);

Since the function isn't passed the section length to perform this check,
could a malformed payload cause print_hex_dump() and subsequent register
accesses to read out of bounds?

> +	if (len < sizeof(*prot_err)) {
> +		pr_err_ratelimited(FW_WARN
> +				   "CXL CPER prot err section too small (%u)\n",
> +				   len);
> +		return -EINVAL;
> +	}
> +

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

Re: [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy

[Alison Schofield]

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>

Re: [PATCH v3 03/10] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy

[Shuai Xue]

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

[PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

[Dave Jiang]

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

Re: [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but ghes_handle_aer() completely misses the fix applied in this patch. It reads 100 bytes from a 96-byte array, triggering a FORTIFY_SOURCE panic, and leaves software-only fields uncleared, leading to a heap information leak.
- [High] This problem wasn't introduced by this patch, but cxl_rch_get_aer_info() calculates MMIO reads using the inflated software struct size, causing an out-of-bounds MMIO read and populating a stack-allocated struct with garbage, driving a stack information leak.
- [High] This problem wasn't introduced by this patch, but aer_recover_queue() fails to free the dynamically allocated aer_capability_regs when the kfifo is full, permanently leaking memory from the critical ghes_estatus_pool.
--

--- Patch [4]: [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer ---
commit 61009f4d553f47557d961ce2fe251ac8b1878dfd
Author: Dave Jiang <dave.jiang@intel.com>
ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

This patch resolves an out-of-bounds read by copying the raw aer_info
buffer into a local struct and clearing the software-only fields.
It prevents hardware data from overwriting header_len and flit.

Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/

> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 06a944dadbc15..fbc88c584c067 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;
[ ... ]
>  	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,

While reviewing this change, I noticed a few related pre-existing issues in
other files that might need similar attention.

drivers/acpi/apei/ghes.c:ghes_handle_aer() {
[ ... ]
		aer_info = (void *)gen_pool_alloc(ghes_estatus_pool,
						  sizeof(struct aer_capability_regs));
		if (!aer_info)
			return;
		memcpy(aer_info, pcie_err->aer_info, sizeof(struct aer_capability_regs));
[ ... ]
}

[Severity: High]
This isn't a bug introduced by this patch, but ghes_handle_aer() completely
misses the fix applied in this patch.

Does this read 100 bytes from the 96-byte pcie_err->aer_info array?

Could this trigger a strict fortify source buffer overflow panic?

Also, since the software-only fields aren't cleared, could
pcie_print_tlp_log() read out of bounds from the heap-allocated dw array,
leaking adjacent ghes_estatus_pool memory into the system log?


drivers/cxl/core/ras_rch.c:cxl_rch_get_aer_info() {
[ ... ]
	int read_cnt = sizeof(struct aer_capability_regs) / sizeof(u32);
	u32 *aer_regs_buf = (u32 *)aer_regs;
	int n;
[ ... ]
	/* Use readl() to guarantee 32-bit accesses */
	for (n = 0; n < read_cnt; n++)
		aer_regs_buf[n] = readl(aer_base + n * sizeof(u32));
[ ... ]
}

[Severity: High]
This is a pre-existing issue, but cxl_rch_get_aer_info() calculates
mmio reads using the inflated software struct size.

Could this cause an out-of-bounds unmapped mmio read, potentially triggering
a machine check exception or unsupported request error?

Additionally, since the aer_regs struct is allocated on the stack in
cxl_handle_rdport_errors(), couldn't reading unmapped mmio fill the flit
and header_len fields with garbage?

If passed to pcie_print_tlp_log(), this might drive an out-of-bounds loop
that leaks kernel stack frames into the system log.


drivers/pci/pcie/aer.c:aer_recover_queue() {
[ ... ]
	if (kfifo_in_spinlocked(&aer_recover_ring, &entry, 1,
				 &aer_recover_ring_lock))
		schedule_work(&aer_recover_work);
	else
		pr_err("buffer overflow in recovery for %04x:%02x:%02x.%x\n",
		       domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
}

[Severity: High]
This is a pre-existing issue, but aer_recover_queue() fails to free the
dynamically allocated aer_capability_regs when the kfifo is full.

Since ghes_handle_aer() allocates this buffer from ghes_estatus_pool and
hands ownership to aer_recover_queue(), does dropping it on the kfifo
overflow error path permanently leak memory from the pool?

Could a burst of pcie aer errors exhaust the pool and break subsequent
ras handling?

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

Re: [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

[Alison Schofield]

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>

Re: [PATCH v3 04/10] ACPI: extlog: Avoid populating software AER metadata from raw hardware buffer

[Shuai Xue]

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

[PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access

[Dave Jiang]

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

Re: [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access

[sashiko-bot]

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

Pre-existing issues:
- [High] The `extlog_print()` function fails to validate the firmware-provided `estatus->data_length` before iterating over its sections, leading to a heap out-of-bounds read.
- [High] Missing `CONFIG_` prefix in the preprocessor guard `#ifdef ACPI_APEI_PCIEAER` renders the error parsing functions and the newly added bounds check dead code.
- [High] The patch fixes an out-of-bounds read vulnerability for PCIe CPER sections in acpi_extlog.c, but leaves identical out-of-bounds reads unpatched in ghes.c.
--

--- Patch [5]: [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access ---
commit aa3cbf45758b1ffcfa8d2f4818d6c1ce97837a2a
Author: Dave Jiang <dave.jiang@intel.com>

ACPI: extlog: Validate PCIe error section length before payload access

This commit adds a missing section-length check before extlog_print_pcie()
touches the PCIe payload to prevent accessing stale adjacent bytes if the
section is too small.

> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index fbc88c584c067..0c440d75d9a78 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

[Severity: High]
This is a pre-existing issue, but does this preprocessor guard lack the
CONFIG_ prefix?

If this should be CONFIG_ACPI_APEI_PCIEAER, this guard evaluates to false,
and both the newly added bounds check and the existing PCIe error parsing
logic will be stripped out as dead code. The CXL protocol error handler
in this file might also have a similar misspelled guard.

>  	struct aer_capability_regs aer_regs = {};
>  	struct aer_capability_regs *aer;
[ ... ]
>  	int aer_severity;
>  	int domain;
>  
> +	if (len < sizeof(*pcie_err))
> +		return;
> +

[Severity: High]
This is a pre-existing issue, but while this patch adds bounds checking
for PCIe CPER sections in acpi_extlog.c, do identical unpatched
out-of-bounds reads exist in drivers/acpi/apei/ghes.c?

Looking at ghes_handle_aer() and ghes_handle_memory_failure() in ghes.c,
they appear to unconditionally access validation_bits without verifying
if gdata->error_data_length is large enough for the struct:

drivers/acpi/apei/ghes.c:ghes_handle_aer() {
    ...
    if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID && ...
    ...
}

Could a short section payload from malicious or corrupted firmware cause
an out-of-bounds read when these GHES handlers are dispatched?

>  	if (!(pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
>  	      pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO))
>  		return;

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

Re: [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access

[Alison Schofield]

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>

Re: [PATCH v3 05/10] ACPI: extlog: Validate PCIe error section length before payload access

[Shuai Xue]

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

[PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion

[Dave Jiang]

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

Re: [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion

[sashiko-bot]

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

Pre-existing issues:
- [High] Preexisting lock inversion (AA deadlock) between process context and IRQ context on CPER work locks.
- [Medium] extlog_print_pcie() is compiled out due to a misspelled preprocessor macro, rendering the function and its newly added bounds check dead code.
--

--- Patch [6]: [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion ---
commit 004b48e5ad5ad98c6645296077a38ad36a959b26
Author: Dave Jiang <dave.jiang@intel.com>

ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion

The commit exports cxl_cper_post_prot_err() from ghes.c and modifies
extlog_print() to use it. This avoids an AB-BA deadlock by deferring CXL
protocol error handling to a workqueue instead of calling it synchronously
while holding the MCE notifier chain rwsem.

Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/

> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 0c440d75d9a78..ae79d090de33b 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
[ ... ]
> @@ -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);
>  

[Severity: Medium]
This isn't a bug introduced by this patch, but does the extlog_print_pcie()
function called for this section type get entirely compiled out?

Looking at extlog_print_pcie() in drivers/acpi/acpi_extlog.c:

drivers/acpi/acpi_extlog.c:extlog_print_pcie() {
#ifdef ACPI_APEI_PCIEAER
    ...
}

Kernel Kconfig macros must use the CONFIG_ prefix to evaluate to true. Since
it checks for ACPI_APEI_PCIEAER instead of CONFIG_ACPI_APEI_PCIEAER, does this
silently disable PCIe AER error logging via the extlog driver and render the
newly added bounds check dead code?

> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 17e4ef5552925..b8dbd99da47ef 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)
>  {

[Severity: High]
This is a pre-existing issue, but can this cause an AA deadlock?

Looking at cxl_cper_register_prot_err_work() and
cxl_cper_unregister_prot_err_work() in drivers/acpi/apei/ghes.c, they acquire
the cxl_cper_prot_err_work_lock using guard(spinlock) without disabling
interrupts since they execute in process context:

drivers/acpi/apei/ghes.c:cxl_cper_register_prot_err_work() {
    ...
    guard(spinlock)(&cxl_cper_prot_err_work_lock);
    ...
}

If an asynchronous hardware error interrupt fires on the same CPU while this
lock is held, the interrupt handler calls cxl_cper_post_prot_err() which
attempts to acquire the exact same lock using guard(spinlock_irqsave):

drivers/acpi/apei/ghes.c:cxl_cper_post_prot_err() {
    ...
    guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
    ...
}

Would this cause the interrupt handler to spin forever waiting for the lock
that the interrupted process context currently holds?

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

Re: [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion

[Alison Schofield]

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>

Re: [PATCH v3 06/10] ACPI: extlog: Defer CXL protocol error handling to avoid lock inversion

[Shuai Xue]

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

[PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

[Dave Jiang]

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

Re: [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

[sashiko-bot]

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

New issues:
- [Medium] The commit message incorrectly claims to fix CXL protocol error handling, but the patch modifies the standard PCIe error handling function.

Pre-existing issues:
- [High] Software-only metadata in `struct aer_capability_regs` is overwritten with unvalidated hardware data, leading to massive out-of-bounds reads during TLP log printing, compounded by out-of-bounds copies from smaller buffers.
- [High] Missing bounds validation on firmware-provided `data_length` allows heap out-of-bounds read in `extlog_print()`.
- [Medium] Bulk-copying the raw hardware `aer_info` buffer into `struct aer_capability_regs` causes a layout mismatch, resulting in Root Port command/status registers being erroneously printed as End-to-End TLP Prefixes.
- [Medium] AER statistics are updated without locking, leading to potential data corruption and lost increments.
--

--- Patch [7]: [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo ---
commit 5975445fed69f606ac825570bf67bde4a1f490fd
Author: Dave Jiang <dave.jiang@intel.com>
Date:   Fri Jul 17 16:16:44 2026 +0000

    ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

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

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

[Severity: Medium]
Is this commit message accurate? The modified function is extlog_print_pcie(),
which parses CPER_SEC_PCIE records (standard PCIe errors), rather than
CPER_SEC_CXL_PROT_ERR (CXL protocol errors).

> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index ae79d090de33b..1fbaf134c04e3 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;

[ ... ]

>  	aer_severity = cper_severity_to_aer(severity);
> 
>  	memcpy(&aer_regs, pcie_err->aer_info, sizeof(pcie_err->aer_info));

[Severity: Medium]
This is a pre-existing issue, but does this bulk copy cause a layout
mismatch? 

The raw hardware aer_info buffer is copied into the Linux struct
aer_capability_regs. Because the software struct embeds a struct pcie_tlp_log
at offset 28, the hardware registers following the 16-byte Header Log might be
erroneously copied into aer_regs.header_log.prefix[]. 

Could this cause pcie_print_tlp_log() to incorrectly print Root Error Command
registers as End-to-End TLP Prefixes?

[Severity: High]
This is a pre-existing issue, but can this overwrite software-only metadata
fields in struct aer_capability_regs?

While this function initializes header_len and flit after the copy, other
functions like ghes_handle_aer() copy 100 bytes (sizeof struct
aer_capability_regs) from a 96-byte hardware buffer, reading memory
out-of-bounds. 

Could unvalidated hardware data corrupting fields like header_len and flit
lead to massive out-of-bounds reads during pcie_print_tlp_log() over the
log->dw[] array?

>  	aer_regs.header_log.header_len = 0;
>  	aer_regs.header_log.flit = false;
>  	aer = &aer_regs;

[Severity: High]
This is a pre-existing issue, but does extlog_print() lack bounds validation
on the firmware-provided data_length?

drivers/acpi/acpi_extlog.c:extlog_print() {
    ...
	memcpy(elog_buf, (void *)estatus, ELOG_ENTRY_LEN);
    ...
	apei_estatus_for_each_section(tmp, gdata) {
    ...
}

Since elog_buf is allocated with a fixed 4096-byte limit, could a
maliciously large data_length from firmware cause the macro to walk off the
end of the heap allocation?

[Severity: Medium]
This is a pre-existing issue, but are the AER statistics updated without
locking?

drivers/pci/pcie/aer.c:pci_dev_aer_stats_incr() {
    ...
    counter[i]++;
    ...
}

Since pci_dev_aer_stats_incr() modifies 64-bit counters concurrently from
multiple execution contexts (like the threaded interrupt handler and
workqueues), could this data race result in corrupted statistics and lost
increments?

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

Re: [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

[Dave Jiang]

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;

Re: [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

[Alison Schofield]

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>

Re: [PATCH v3 07/10] ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo

[Shuai Xue]

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

[PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access

[Dave Jiang]

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

Re: [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing payload length bounds checks for PCIe error sections before payload access in `ghes_handle_aer()`.
- [High] Missing payload length validation in vendor-specific HiSilicon PCIe error handler causes out-of-bounds read.
- [High] The `queued` status for synchronous errors is blindly overwritten during multi-section generic status blocks.
- [High] Unconditional out-of-bounds read in `cper_mem_err_pack()` for 73-byte legacy memory error records.
- [High] Short memory error records with manipulated validation bits will cause consumers to act on out-of-bounds data.
--

--- Patch [8]: [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access ---
commit 1aac797f010b3f5d2062204450ea2f731ee9a1d5
Author: Dave Jiang <dave.jiang@intel.com>

ACPI: APEI: GHES: Validate memory error section length before payload access

This commit adds a payload length validation for memory error sections
in ghes_do_proc() before allowing report chain consumers to access the
data. The check ensures the length is at least the size of the older
UEFI 2.1/2.2 layout to prevent out-of-bounds reads on truncated
records.

Link: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=7

> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index b8dbd99da47ef..1d2966a437bdb 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.
> +			 */

[Severity: High]
This isn't a bug introduced by this patch, but it looks like short memory
error records with manipulated validation bits could cause consumers to act
on out-of-bounds data.

The commit message states that trailing fields are only acted on under
validation bits that a short record leaves clear. However, does the length
check in ghes_do_proc() enforce this correlation?

If firmware provides a 73-byte record but sets the validation bits for the
extended fields (e.g., CPER_MEM_VALID_MODULE_HANDLE), will consumers like
ghes_edac and the mc_event tracepoint trust the validation bits and actively
consume the out-of-bounds data packed by cper_mem_err_pack()?

> +			if (gdata->error_data_length <
> +			    sizeof(struct cper_sec_mem_err_old))
> +				continue;

[Severity: High]
This isn't a bug introduced by this patch, but explicitly allowing 73-byte
legacy memory error records appears to cause an unconditional out-of-bounds
read downstream.

The payload is passed to ghes_report_chain consumers, such as ghes_edac.
When ghes_edac_report_mem_error() calls cper_mem_err_pack():

drivers/firmware/efi/cper.c:cper_mem_err_pack() {
    ...
    cmem->extended = mem->extended;
    cmem->rank = mem->rank;
    cmem->mem_array_handle = mem->mem_array_handle;
    cmem->mem_dev_handle = mem->mem_dev_handle;
    ...
}

Does this unconditionally copy the trailing 7 bytes? If firmware provides a
73-byte record as permitted by this new check, does this cause an
unconditional out-of-bounds read past the end of the CPER section?

> +
>  			atomic_notifier_call_chain(&ghes_report_chain, sev, mem_err);
>  
>  			arch_apei_report_mem_error(sev, mem_err);

[Severity: High]
This isn't a bug introduced by this patch, but the queued status for
synchronous errors seems to be blindly overwritten during multi-section
generic status blocks.

In ghes_do_proc(), right after handling the memory error:

drivers/acpi/apei/ghes.c:ghes_do_proc() {
    ...
        queued = ghes_handle_memory_failure(gdata, sev, sync);
    } else if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) {
        queued = ghes_handle_arm_hw_error(gdata, sev, sync);
    ...
}

If a generic status block contains both a CPER_SEC_PLATFORM_MEM and a
CPER_SEC_PROC_ARM section, does the ARM handler's return value overwrite the
MEM handler's return value? If the MEM error cannot be queued, but the ARM
error is successfully queued, queued becomes true. Could this incorrectly
bypass the force_sig(SIGBUS) fallback for unhandled synchronous errors,
potentially causing an infinite exception loop upon returning to userspace?

[Severity: High]
This isn't a bug introduced by this patch, but missing payload length bounds
checks for PCIe error sections before payload access in ghes_handle_aer() can
lead to out-of-bounds reads.

When ghes_handle_aer() accesses the payload:

drivers/acpi/apei/ghes.c:ghes_handle_aer() {
    ...
    struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata);
    
    if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID && ...
    ...
}

Does this read pcie_err->validation_bits without checking
gdata->error_data_length? If the firmware provides a truncated PCIe section,
could this read past the bounds? If the out-of-bounds memory contains
valid-looking bits, could it copy 96 bytes of aer_info from out-of-bounds
memory into the AER recovery queue, potentially leaking kernel pool memory or
corrupting AER state?

[Severity: High]
This isn't a bug introduced by this patch, but there is a missing payload
length validation in the vendor-specific HiSilicon PCIe error handler.

When hisi_pcie_notify_error() accesses the payload:

drivers/pci/controller/pcie-hisi-error.c:hisi_pcie_notify_error() {
    ...
    const struct hisi_pcie_error_data *error_data = acpi_hest_get_payload(gdata);
    ...
    if (error_data->socket_id != socket)
        return NOTIFY_DONE;
    ...
}

Does this cast the raw payload and read error_data->socket_id without any
length validation? If the section is empty or truncated, could this read past
the end of the pool allocation? If socket_id randomly matches, could it
further read up to 132 bytes of out-of-bounds memory based on out-of-bounds
val_bits, leaking kernel pool memory?

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

Re: [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access

[Alison Schofield]

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>

Re: [PATCH v3 08/10] ACPI: APEI: GHES: Validate memory error section length before payload access

[Shuai Xue]

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

[PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata

[Dave Jiang]

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

Re: [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata

[sashiko-bot]

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

Pre-existing issues:
- [High] Memory allocated for AER registers from the NMI-safe pool is leaked when the aer_recover_ring kfifo is full.
- [High] Memory allocated for AER registers from the NMI-safe pool is leaked when the target PCI device cannot be found during asynchronous recovery.
- [High] The CXL RCH driver reads past the physical AER capability into a stack-allocated struct, corrupting software metadata and triggering out-of-bounds stack memory accesses.
--

--- Patch [9]: [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata ---
commit 4a70c73fe1d51d5ff3adef3bd4739dd98b4c3b8d
Author: Dave Jiang <dave.jiang@intel.com>

ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
    
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.

Link: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=3

> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 1d2966a437bdb..bd53509dcab3c 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c

[ ... ]

> @@ -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;

[Severity: High]
This isn't a bug introduced by this patch, but given the sizing fixes here
regarding struct aer_capability_regs, does the CXL RCH driver have a similar
out-of-bounds access?

In cxl_rch_get_aer_info(), the loop reads from MMIO based on the software
struct size rather than the hardware capability:

drivers/cxl/core/ras_rch.c:cxl_rch_get_aer_info() {
    ...
    for (n = 0; n < read_cnt; n++)
        aer_regs_buf[n] = readl(aer_base + n * sizeof(u32));
    ...
}

Reading past the capability can return 0xFFFFFFFF, which would overwrite
header_len to 0xFF and flit to true in the stack-allocated aer_regs.

When passed to pci_print_aer() in cxl_handle_rdport_errors(), could this
corrupted header_len cause pcie_print_tlp_log() to read far out-of-bounds
on the stack?

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

[Severity: High]
This is a pre-existing issue, but does this code leak the memory allocated
from ghes_estatus_pool if the aer_recover_ring kfifo is full?

If the kfifo insertion fails in aer_recover_queue():

drivers/pci/pcie/aer.c:aer_recover_queue() {
    ...
    if (kfifo_in_spinlocked(&aer_recover_ring, &entry, 1,
                             &aer_recover_ring_lock))
        schedule_work(&aer_recover_work);
    else
        pr_err("buffer overflow in recovery for %04x:%02x:%02x.%x\n",
               domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
}

The function returns void without freeing the aer_regs pointer. Could this
deplete the NMI-safe ghes_estatus_pool during a burst of AER events?

[Severity: High]
This is also a pre-existing issue, but does this same allocation leak
during asynchronous recovery if the PCI device is not found?

When the worker processes the queue in aer_recover_work_func():

drivers/pci/pcie/aer.c:aer_recover_work_func() {
    ...
    if (!pdev) {
        pr_err_ratelimited("%04x:%02x:%02x.%x: no pci_dev found\n",
                           entry.domain, entry.bus,
                           PCI_SLOT(entry.devfn),
                           PCI_FUNC(entry.devfn));
        continue;
    }
    ...
}

If the device lookup fails, the continue statement bypasses the
ghes_estatus_pool_region_free() call at the end of the loop, which
would leave the memory chunk unreleased.

>  	}
>  #endif
>  }

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

Re: [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata

[Alison Schofield]

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>

Re: [PATCH v3 09/10] ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata

[Shuai Xue]

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

[PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections

[Dave Jiang]

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

Re: [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections

[sashiko-bot]

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

Pre-existing issues:
- [High] The newly added bounds check using `cper_estatus_len()` can be bypassed due to an integer truncation, failing to prevent the out-of-bounds read it was intended to fix.
- [High] An integer overflow in `cper_estatus_check_header()` allows malformed records to bypass the bounds check on 32-bit systems.
- [High] The physical address supplied by firmware is not validated to fall within the memory-mapped `elog_addr` region, allowing out-of-bounds memory accesses.
- [High] The `fru_text` string field within the CPER record is missing a guaranteed null-terminator before being passed to trace events, causing `strlen()` to read out of bounds.
--

--- Patch [10]: [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections ---
commit a0127a0e48d0e90f30ee3978c528f569d5e624af
Author: Dave Jiang <dave.jiang@intel.com>
Subject: ACPI: extlog: Validate elog record length before walking sections

This commit attempts to fix an out-of-bounds read by adding bounds checks
on the firmware-controlled data_length before walking the elog sections.
Malformed records exceeding ELOG_ENTRY_LEN are now dropped with NOTIFY_DONE
instead of being processed.

Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=6

> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 1fbaf134c04e3..30863fd3f5b07 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))

[Severity: High]
This is a pre-existing issue in the helper function, but does the 32-bit
truncation in cper_estatus_len() bypass this newly added bounds check?

Looking at cper_estatus_len():

drivers/acpi/apei/apei-internal.h:cper_estatus_len() {
    ...
    return sizeof(*estatus) + estatus->data_length;
}

If a malformed firmware record sets data_length to an extremely large value
like 0xFFFFFFFF, the 64-bit sum evaluates to 0x100000013. Because the
function returns a 32-bit u32, this is implicitly truncated to 19.

Since 19 is not greater than ELOG_ENTRY_LEN (4096), the check passes, and
cper_estatus_check() will validate and loop over the un-truncated large
data_length, allowing the parser to read out of bounds.


[Severity: High]
This is a pre-existing issue, but can malformed records bypass the bounds
check in cper_estatus_check_header() on 32-bit systems?

Looking at cper_estatus_check_header(), which is called by the new
cper_estatus_check() validation:

drivers/firmware/efi/cper.c:cper_estatus_check_header() {
    ...
    if (estatus->raw_data_length &&
        estatus->raw_data_offset < sizeof(*estatus) + estatus->data_length)
        return -EINVAL;
}

On 32-bit architectures, if data_length is close to the 32-bit maximum (e.g.,
0xFFFFFFEC), the calculation sizeof(*estatus) + estatus->data_length
overflows to a small value (e.g., 0).

The safety check then evaluates to false and passes, allowing the large
data_length to be used in section parsing and walking past the bounds of the
buffer.

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


While reviewing the rest of the file, I noticed a few other pre-existing 
issues:

[Severity: High]
This is a pre-existing issue, but could a corrupted firmware physical address
lead to accessing arbitrary kernel memory?

In drivers/acpi/acpi_extlog.c:extlog_elog_entry_check():

    data &= EXT_ELOG_ENTRY_MASK;
    estatus = (struct acpi_hest_generic_status *)ELOG_ENTRY_ADDR(data);

    /* if no valid data in elog entry, just return */
    if (estatus->block_status == 0)

The physical address data is read directly from the firmware-controlled L1
directory and translated into a virtual address via pointer arithmetic against
the elog_addr mapping.

There appears to be no bounds check to ensure the address falls within the
intended elog_base and elog_size region. If firmware supplies a corrupted
address, the resulting pointer will be out-of-bounds, causing a potential
kernel panic upon dereferencing block_status.


[Severity: High]
This is a pre-existing issue, but is it possible for the tracepoint to read
out of bounds when processing the non-terminated fru_text string?

In drivers/acpi/acpi_extlog.c:extlog_print():

    if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
        fru_text = gdata->fru_text;
    ...
    trace_extlog_mem_event(mem, err_seq, fru_id, fru_text,
                           (u8)gdata->error_severity);

The fru_text string is extracted from a fixed 20-byte array without
enforcing a null terminator.

Since the tracepoint implicitly calls strlen() on fru_text, a firmware
record filled with non-null characters could cause strlen() to read out of
bounds into adjacent kernel heap memory, leaking memory contents to the
tracing infrastructure.

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

Re: [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections

[Alison Schofield]

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>

Re: [PATCH v3 10/10] ACPI: extlog: Validate elog record length before walking sections

[Shuai Xue]

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

Re: [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko

[Rafael J. Wysocki (Intel)]

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.

Re: [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko

[Dave Jiang]

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. 

Re: [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko

[Rafael J. Wysocki (Intel)]

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.