Re: [PATCH v4 03/13] ACPI: extlog: Validate elog record length before walking sections

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 24 Aug 2026 10:49:26 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> extlog_print() copies a fixed ELOG_ENTRY_LEN (4096) bytes from the elog
> record into elog_buf, then walks the sections using the firmware-controlled
> data_length. Nothing keeps data_length inside the buffer, so a malformed
> record walks the section pointer past elog_buf and reads adjacent memory.
> Unlike the GHES paths, extlog never calls cper_estatus_check().
> 
> Reject a record longer than ELOG_ENTRY_LEN and run cper_estatus_check()
> before walking the sections. The length test alone is not enough: a wrapped
> length reads back short and passes it, which cper_estatus_check() catches
> via the header check added earlier. Drop a malformed record with
> NOTIFY_DONE and without MCE_HANDLED_EXTLOG, since extlog did not consume
> it.
> 
> Reported-by: sashiko-bot@kernel.org
> Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=6
> Fixes: f6ec01da40e4 ("ACPI: extlog: Handle multiple records")
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/acpi/acpi_extlog.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> index 7ad3b36013cc..9ad0052aa20c 100644
> --- a/drivers/acpi/acpi_extlog.c
> +++ b/drivers/acpi/acpi_extlog.c
> @@ -208,6 +208,10 @@ static int extlog_print(struct notifier_block *nb, unsigned long val,
>  
>  	tmp = (struct acpi_hest_generic_status *)elog_buf;
>  
> +	/* Keep the firmware-controlled data_length inside elog_buf. */
> +	if (cper_estatus_len(tmp) > ELOG_ENTRY_LEN || cper_estatus_check(tmp))

Why this order?  To me checking if we are in crazy world (overflow) before
doing anything with the overflowed value makes more sense. So that would be swapping
the two conditions.

> +		return NOTIFY_DONE;
> +
>  	if (!ras_userspace_consumers()) {
>  		print_extlog_rcd(NULL, tmp, cpu);
>  		goto out;