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

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: [email protected]
> Closes: https://sashiko.dev/#/patchset/[email protected]?part=1
> Fixes: 45b14a4ffcc1 ("efi: cper: Fix possible out-of-bounds access")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <[email protected]>
> ---
> 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 <[email protected]>

Thanks.
Shuai