Re: [PATCH 2/4] ACPI: APEI: GHES: Bound CXL event record copy to the firmware section length

[sashiko-bot@kernel.org]

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

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

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

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

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

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

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

[ ... ]

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

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

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

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

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

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

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

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

>  		} else if (guid_equal(sec_type, &CPER_SEC_CXL_GEN_MEDIA_GUID)) {
>  			struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
>  
> -			cxl_cper_post_event(CXL_CPER_EVENT_GEN_MEDIA, rec);
> +			cxl_cper_post_event(CXL_CPER_EVENT_GEN_MEDIA, rec,
> +					    gdata->error_data_length);
>  		} else if (guid_equal(sec_type, &CPER_SEC_CXL_DRAM_GUID)) {
>  			struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata);
>  
> -			cxl_cper_post_event(CXL_CPER_EVENT_DRAM, rec);
> +			cxl_cper_post_event(CXL_CPER_EVENT_DRAM, rec,
> +					    gdata->error_data_length);

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