Re: [PATCH v5 3/9] mm/memory-failure: libstub: install the poisoned-memory EFI table

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: efi: integer overflow in poisoned memory bitmap allocation on 32-bit
--

commit 591eef51f672049b14b50a75d67dac823925a19a
Author: Breno Leitao <leitao@xxxxxxxxxx>

mm/memory-failure: libstub: install the poisoned-memory EFI table

This commit introduces a new poisoned-memory EFI table installed by the stub.
It sizes a bitmap based on the UEFI memory map to describe the RAM span and
allocates it as EFI_ACPI_RECLAIM_MEMORY so the next kernel does not take it
as free RAM.

> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 48f93f7758e9e..9c66e06c972c5 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c

[ ... ]

> +static struct linux_efi_poisoned_memory *efi_poison_alloc(u64 phys_base,
> +							  u64 bitmap_size)
> +{
> +	struct linux_efi_poisoned_memory *pm;
> +	efi_status_t status;
> +
> +	status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY,
> +			     sizeof(*pm) + bitmap_size, (void **)&pm);

[Severity: High]
Could this allocation size wrap around on 32-bit EFI architectures if the
memory map is extremely sparse?

In efi_get_ram_range(), ram_top is determined from EFI memory map
descriptors. If a descriptor is placed at a very high physical address,
bitmap_size could be large enough to wrap around 32-bit arithmetic.

On 32-bit EFI, the allocation size passed to allocate_pool (which takes
a 32-bit UINTN) is implicitly truncated. For example, if bitmap_size modulo
2^32 is very large (e.g., 0xFFFFFFF0), sizeof(*pm) + bitmap_size could
truncate to a tiny allocation (e.g., 8 bytes).

> +	if (status != EFI_SUCCESS)
> +		return NULL;
> +
> +	pm->version = 1;
> +	pm->unit_size = EFI_POISON_UNIT_SIZE;
> +	pm->phys_base = phys_base;
> +	pm->size = bitmap_size;
> +	memset(pm->bitmap, 0, bitmap_size);

[Severity: High]
If the allocation size wrapped around, could this memset cause a heap
buffer overflow?

The 64-bit bitmap_size is passed to memset, which takes a 32-bit size_t on
32-bit architectures. This independently truncates bitmap_size (e.g., to
0xFFFFFFF0). Since the prior allocation could have wrapped to a very small
size, this would write gigabytes of zeroes into a tiny allocation, causing
boot-time memory corruption.

Would it be safer to check if bitmap_size exceeds the maximum allocation
size for the architecture, or use a safe addition helper before allocating?

> +
> +	return pm;
> +}
> +

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