Re: [PATCH v5 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] Missing lower bounds check for the section header in `cper_estatus_check()` allows out-of-bounds reads when iterating truncated CPER records.
--

commit 2032c31f790eb3dad96643cacd189ae773ea581e
Author: Dave Jiang <dave.jiang@intel.com>
efi/cper: Reject CPER records with an out-of-range error_data_length

This commit modifies the size validation in cper_estatus_check to use
check_add_overflow. It addresses an issue where a large error_data_length
could wrap around as a signed int, bypassing the original size checks and
causing an infinite loop or out-of-bounds access.

Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=1
Fixes: 45b14a4ffcc1 ("efi: cper: Fix possible out-of-bounds access")

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

[Severity: High]
This is a pre-existing issue, but does this code risk an out-of-bounds read
if the CPER record is truncated?

The apei_estatus_for_each_section loop condition only checks if the start of
the section is within estatus->data_length.

If a truncated record leaves fewer than sizeof(struct acpi_hest_generic_data)
bytes for the final section, the loop will still iterate.

Inside the loop, acpi_hest_get_size(gdata) reads gdata->revision, and
check_add_overflow reads gdata->error_data_length before confirming if
data_len actually has enough space for the struct header itself.

Could this result in page faults or memory corruption if malformed firmware
leaves us with a truncated section header?

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