Re: [PATCH v6 2/7] mm/memory-failure: libstub: install the poisoned-memory EFI table

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: 32-bit integer truncation in efi_poison_alloc causes heap buffer overflow
--

commit 09b9f9b59673eeb21856d3365898ea89a642bbc0
Author: Breno Leitao <leitao@debian.org>

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

This commit adds support for installing a poisoned-memory EFI configuration
table during boot services. It sizes a bitmap based on the RAM span to
track memory failures.

> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
[ ... ]
> +static efi_status_t efi_get_ram_range(u64 *base, u64 *top)
> +{
[ ... ]
> +	if (!ram_top || ram_base == ULLONG_MAX)
> +		return EFI_NOT_FOUND;
> +
> +	*base = round_down(ram_base, EFI_POISON_UNIT_SIZE);
> +	*top = round_up(ram_top, EFI_POISON_UNIT_SIZE);

[Severity: High]
Could ram_top wrap around to 0 here if it is very close to ULLONG_MAX?

If ram_top wraps to 0, the later calculation of span (ram_top - ram_base)
in install_poisoned_memory_table() would result in a very large 64-bit value.

> +
> +	return EFI_SUCCESS;
> +}
[ ... ]
> +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]
Does this risk a heap buffer overflow due to 32-bit integer truncation?

If the earlier calculation produces a bitmap_size exceeding 32 bits, could
sizeof(*pm) + bitmap_size truncate to a tiny allocation on 32-bit
architectures?

The subsequent memset() might then use the truncated or original 64-bit size
to write far past the bounds of the allocated pool:

    memset(pm->bitmap, 0, bitmap_size);

> +	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);
> +
> +	return pm;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-hwpoison-kho-v6-0-f304255147da@debian.org?part=2