Re: [PATCH v6 11/13] ACPI: APEI: GHES: Validate memory error section length before payload access

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch fixes length validation in ghes_do_proc() but leaves an identical logic flaw and buffer over-read unfixed in cper_print_mem().
--

commit 8bbaca1beceadb4f3349da69c4f0bedd1d913c54
Author: Dave Jiang <dave.jiang@intel.com>
Subject: ACPI: APEI: GHES: Validate memory error section length before payload access

This patch adds length validation in ghes_do_proc() before accessing the
CPER_SEC_PLATFORM_MEM payload to prevent reading past the record. It ensures
records are at least 73 bytes and validates the remaining length based on the
claimed fields.

Closes: https://sashiko.dev/#/patchset/20260714231835.303081-1-dave.jiang@intel.com?part=7

> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 111895c57c2f9..2077cc90569a0 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
[ ... ]
> @@ -954,6 +976,25 @@ static void ghes_do_proc(struct ghes *ghes,
>  		if (guid_equal(sec_type, &CPER_SEC_PLATFORM_MEM)) {
>  			struct cper_sec_mem_err *mem_err = acpi_hest_get_payload(gdata);
>  
> +			/*
> +			 * Check once for all three consumers below. The 73-byte
> +			 * UEFI 2.1/2.2 layout is the floor, matching
> +			 * cper_estatus_print_section() and making
> +			 * validation_bits safe to read.
> +			 */
> +			if (gdata->error_data_length <
> +			    sizeof(struct cper_sec_mem_err_old))
> +				continue;
> +

[Severity: High]
This isn't a bug introduced by this patch, but does leaving cper_print_mem()
unfixed bypass these newly added safety checks?

As noted in the commit message, cper_print_mem() gets the mask checking
wrong. When firmware provides a CPER memory error record with a length
between 74 and 79 bytes, the exact match check in cper_print_mem() fails to
catch the invalid length:

drivers/firmware/efi/cper.c:cper_print_mem() {
    ...
	/* Don't trust UEFI 2.1/2.2 structure with bad validation bits */
	if (len == sizeof(struct cper_sec_mem_err_old) &&
	    (mem->validation_bits & ~(CPER_MEM_VALID_RANK_NUMBER - 1))) {
		pr_err(FW_WARN "valid bits set for fields beyond structure\n");
		return;
	}
    ...
}

Because ghes_proc() calls ghes_print_estatus() before ghes_do_proc(), this
code executes before the new checks in this patch are reached.

When cper_print_mem() then calls cper_mem_err_pack(), it blindly reads
out-of-bounds fields if the payload is smaller than 80 bytes:

drivers/firmware/efi/cper.c:cper_mem_err_pack() {
    ...
	if (mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE)
		cmem->mem_dev_handle = mem->mem_dev_handle;
    ...
}

Could this result in an out-of-bounds memory read (such as mem_dev_handle
at offset 78) and an information leak to dmesg before ghes_do_proc() has a
chance to reject the malformed record?

> +			/* Then require what the claimed fields actually need. */
> +			if (gdata->error_data_length <
> +			    ghes_mem_err_min_len(mem_err->validation_bits)) {
> +				pr_warn_ratelimited(FW_WARN GHES_PFX
> +						    "memory error section too small (%u) for the fields it claims\n",
> +						    gdata->error_data_length);
> +				continue;
> +			}
> +
>  			atomic_notifier_call_chain(&ghes_report_chain, sev, mem_err);
>  
>  			arch_apei_report_mem_error(sev, mem_err);

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