Re: [PATCH v7 04/10] ACPI: APEI: GHES: move estatus cache helpers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Malicious hardware or firmware can cause a memory leak in the fixed-size `ghes_estatus_pool` by reducing the CPER error status length between the peek and full read operations.
--

--- Patch [4]: [PATCH v7 04/10] ACPI: APEI: GHES: move estatus cache helpers ---
commit 838eea76a409eda9e1dc7c8c377f46dbbbad493c
Author: Ahmed Tiba <ahmed.tiba@arm.com>

ACPI: APEI: GHES: move estatus cache helpers

This patch relocates the estatus cache allocation and lookup helpers from
ghes.c into ghes_cper.c. It also fixes a bug in ghes_estatus_cache_add()
to avoid memory leaks and counter exhaustion.

[ ... ]

> diff --git a/include/acpi/ghes_cper.h b/include/acpi/ghes_cper.h
> index 4649e3140888c..15305c8be9a74 100644
> --- a/include/acpi/ghes_cper.h
> +++ b/include/acpi/ghes_cper.h
> @@ -55,6 +55,8 @@
>  	((struct acpi_hest_generic_data *)                              \
>  	((struct ghes_vendor_record_entry *)(vendor_entry) + 1))
>  
> +extern struct gen_pool *ghes_estatus_pool;

[Severity: High]
This is a pre-existing issue, but does the system have a memory leak in
ghes_estatus_pool if the hardware or firmware changes the CPER error status
length between the peek and full read operations?

If malicious or compromised hardware reduces the length mid-read, it seems we
can permanently leak memory in the fixed-size pool:

1. ghes_in_nmi_queue_one_entry() allocates from ghes_estatus_pool based on
   the original length from the initial hardware peek.
2. __ghes_read_estatus() performs a second fetch from hardware memory. It
   accepts the new length if it is smaller than the original, persisting it
   in kernel RAM.
3. ghes_proc_in_irq() later calculates the free size using
   cper_estatus_len(estatus) on the copied memory.

Since the length is now smaller, gen_pool_free() would free fewer bytes than
were originally allocated. Could this eventually exhaust the pool and prevent
the kernel from queuing further NMI-based hardware errors?

> +
>  static inline bool is_hest_type_generic_v2(struct ghes *ghes)
>  {
>  	return ghes->generic->header.type == ACPI_HEST_TYPE_GENERIC_ERROR_V2;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-topics-ahmtib01-ras_ffh_arm_internal_review-v7-0-8b3a85216cef@arm.com?part=4