Re: [PATCH v6 01/13] efi/cper: Reject CPER records with an out-of-range error_data_length

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The section iterator allows out-of-bounds memory reads when evaluating the generic data header on truncated CPER records.
--

commit 11208300cad76f5a8ea35d8ac53a2941f6447499
Author: Dave Jiang <dave.jiang@intel.com>

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

This commit uses check_add_overflow() to reject CPER records with a huge
error_data_length that wraps around when summed with the header size. It
prevents infinite loops or out-of-bounds walks caused by integer overflow
during section iteration.

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..332c78f2d5617 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c

[ ... ]

> @@ -762,10 +762,18 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
>  	data_len = estatus->data_length;
>  
>  	apei_estatus_for_each_section(estatus, gdata) {
> -		if (acpi_hest_get_size(gdata) > data_len)
> +		int record_size;
> +
> +		/*
> +		 * The <acpi/ghes.h> helpers sum these as a signed int, so a
> +		 * huge error_data_length wraps small rather than large and the
> +		 * walk then advances by that wrapped value. Reject a size an
> +		 * int cannot carry.
> +		 */
> +		if (check_add_overflow(acpi_hest_get_size(gdata),
> +				       gdata->error_data_length, &record_size))

[Severity: High]
This isn't a bug introduced by this patch, but does the section iterator
allow out-of-bounds memory reads when evaluating the generic data header on
truncated CPER records?

If the firmware provides a CPER record where the total data length covers
the first section plus a small positive remainder (for example, 1 byte), the
apei_estatus_for_each_section() loop will advance to the second section
because there is still remaining data length.

Inside the loop, the updated code evaluates check_add_overflow():

    if (check_add_overflow(acpi_hest_get_size(gdata),
                           gdata->error_data_length, &record_size))

This evaluates acpi_hest_get_size(gdata) (which accesses gdata->revision)
and reads gdata->error_data_length before verifying that the remaining data
length is large enough to contain the generic data header.

Could this trigger a page fault and kernel panic in NMI or IRQ context if
the firmware error logs are malformed? While this problem was pre-existing,
this patch replaces the acpi_hest_get_size() check with check_add_overflow(),
adding an unconditional read of gdata->error_data_length to the existing
out-of-bounds read of gdata->revision.

>  			return -EINVAL;
>  
> -		record_size = acpi_hest_get_record_size(gdata);
>  		if (record_size > data_len)
>  			return -EINVAL;
>

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