Re: [PATCH v5 4/9] mm/memory-failure: efi: adopt the inherited poisoned-memory table

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] efi: ABI incompatibility in poisoned-memory table due to unsigned long
- [High] efi: ABI incompatibility in poisoned-memory table due to PAGE_SIZE check
--

commit 213ea31d156bf79dbf85ad9fc8678fe09bddbde2
Author: Breno Leitao <leitao@debian.org>

mm/memory-failure: efi: adopt the inherited poisoned-memory table

This commit takes the inherited EFI poisoned-memory table into use by parsing
it in efi_config_parse_tables(). It validates the table header and hands its
pages to memblock so everything afterwards can reach it with phys_to_virt().

> diff --git a/drivers/firmware/efi/poison.c b/drivers/firmware/efi/poison.c
> new file mode 100644
> index 0000000000000..3f12db3dc9b84
> --- /dev/null
> +++ b/drivers/firmware/efi/poison.c
> @@ -0,0 +1,93 @@
[ ... ]
> +static bool __init
> +efi_poison_geometry_valid(const struct linux_efi_poisoned_memory *pm)
> +{
> +	u64 nbits, end;
> +
> +	/* Whole words, and a bit count that can be taken without wrapping. */
> +	if (!pm->size || !IS_ALIGNED(pm->size, sizeof(unsigned long)) ||
> +	    check_mul_overflow(pm->size, (u64)BITS_PER_BYTE, &nbits))
> +		return false;

[Severity: High]
Does this validation logic break binary compatibility across different kernel
bitnesses by relying on unsigned long?

The ABI structure definition for linux_efi_poisoned_memory uses a
variable-width type for the bitmap (unsigned long bitmap[]) rather than a
fixed-width type like u64. If a 32-bit kernel (or 32-bit EFI environment)
creates the table with a size that is a multiple of 4 (but not 8) and
passes it via kexec to a 64-bit kernel, this validation will fail since
sizeof(unsigned long) is 8.

The 64-bit consumer kernel will reject the valid poisoned-memory table,
losing hardware memory poison information. Accessing the unpoisoned memory
subsequently triggers a Machine Check Exception (MCE), crashing the system.

> +
> +	/* And a footprint that can be page aligned without wrapping either. */
> +	if (check_add_overflow(efi.poisoned_memory, sizeof(*pm) + pm->size,
> +			       &end) || end > PHYS_ADDR_MAX - PAGE_SIZE)
> +		return false;
> +
> +	if (pm->unit_size < PAGE_SIZE || !is_power_of_2(pm->unit_size))
> +		return false;

[Severity: High]
Can this logic break kexec chains by tying the acceptance of an external ABI
table to the consumer's local compile-time PAGE_SIZE?

If a kernel configured with a smaller PAGE_SIZE (e.g., 4KB) sets
pm->unit_size to 4096 and kexecs into a kernel configured with a larger
PAGE_SIZE (e.g., 64KB), the new kernel evaluates 4096 < 65536 and entirely
rejects the table.

Discarding all hardware memory poison records can result in MCEs and system
crashes when the supposedly good memory is allocated and accessed. Should
the consumer instead accept the table and safely round up poisoned ranges to
its native page granularity?

> +
> +	return IS_ALIGNED(pm->phys_base, pm->unit_size);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-hwpoison-kho-v5-0-3bc7a57bd503@debian.org?part=4