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

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: [email protected]
>> 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 <[email protected]>
>> ---
>>   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