Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing overflow check when computing the end address of the EFI poisoned memory table can lead to an unbounded memblock reservation and memory corruption. --
commit 7216d7e029930900ac5741a5f5bbfe7a2107b408 Author: Breno Leitao <[email protected]> mm/memory-failure: efi: record hardware-poisoned frames into the poisoned-memory table This commit hooks action_result() to efi_hwpoison_record_pfn() to record hard offlined frames in the LINUX_EFI_POISONED_MEMORY table, ensuring they are passed down to the next kernel via kexec. It also adds early initialization code to vet the table inherited from previous boots. > diff --git a/drivers/firmware/efi/poison.c b/drivers/firmware/efi/poison.c > new file mode 100644 > index 0000000000000..c18edf111c710 > --- /dev/null > +++ b/drivers/firmware/efi/poison.c [ ... ] > +static bool __init > +efi_poison_geometry_valid(const struct linux_efi_poisoned_memory *pm) > +{ > + u64 nbits; > + > + /* 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 fully protect against an excessively large pm->size? While check_mul_overflow() validates the size for bit conversion, it appears it does not verify that the table's footprint fits within the physical address space without overflowing when combined with efi.poisoned_memory. > + > + if (pm->unit_size < PAGE_SIZE || !is_power_of_2(pm->unit_size)) > + return false; > + > + return IS_ALIGNED(pm->phys_base, pm->unit_size); > +} [ ... ] > +void __init efi_poisoned_memory_reserve(void) > +{ > + struct linux_efi_poisoned_memory *pm; > + phys_addr_t start, end; > + > + if (efi.poisoned_memory == EFI_INVALID_TABLE_ADDR) > + return; > + > + pm = early_memremap(efi.poisoned_memory, sizeof(*pm)); > + if (!pm) { > + pr_warn("Could not map poisoned-memory table\n"); > + efi.poisoned_memory = EFI_INVALID_TABLE_ADDR; > + return; > + } > + > + if (!efi_poison_table_valid(pm)) { > + efi.poisoned_memory = EFI_INVALID_TABLE_ADDR; > + early_memunmap(pm, sizeof(*pm)); > + return; > + } > + > + start = PAGE_ALIGN_DOWN(efi.poisoned_memory); > + end = PAGE_ALIGN(efi.poisoned_memory + sizeof(*pm) + pm->size); [Severity: High] Can this computation wrap around if pm->size is maliciously large? Because pm->size is only constrained to be less than U64_MAX / 8 in efi_poison_geometry_valid(), an unvalidated addition here could overflow, resulting in end < start. If that happens, an enormous end - start would be passed to memblock_add() and memblock_reserve(). This could result in memblock capping the size to PHYS_ADDR_MAX - start, blindly marking the rest of the physical address space (including MMIO) as RAM. This forces direct-map cacheable page table entries for MMIO regions, leading to system crashes or memory layout corruption. > + early_memunmap(pm, sizeof(*pm)); > + > + memblock_add(start, end - start); > + memblock_reserve(start, end - start); > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
