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 <[email protected]> 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/[email protected]?part=3
