Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] mm: TOCTOU race in memblk_nr_poison_init() drops early MCE counts --
commit a8282f47e9b2f9ae014e9948a2edfc336de13cd0 Author: Breno Leitao <[email protected]> drivers/base/memory: count inherited poisoned frames into the block This commit initializes the poisoned frame count for memory blocks created during early boot by scanning for pre-existing poisoned pages. It addresses an issue where kexec-inherited poisoned frames would later cause a counter underflow on unpoisoning, preventing the block from being onlined. > diff --git a/drivers/base/memory.c b/drivers/base/memory.c > index 5eead3346f1e3..49a33ddbb2e71 100644 > --- a/drivers/base/memory.c > +++ b/drivers/base/memory.c [ ... ] > @@ -807,6 +812,7 @@ static int add_memory_block(unsigned long block_id, int > nid, unsigned long state > mem->state = state; > mem->nid = nid; > INIT_LIST_HEAD(&mem->group_next); > + memblk_nr_poison_init(mem); > > #ifndef CONFIG_NUMA > if (state == MEM_ONLINE) [Severity: Medium] Could this ordering create a Time-Of-Check to Time-Of-Use (TOCTOU) race during early boot? If a hardware machine check exception (MCE) occurs concurrently after memblk_nr_poison_init() runs but before __add_memory_block() publishes the block via xa_store(), does the memory_failure() worker set the page flag but drop the block counter increment because memory_block_get() still returns NULL? If this concurrent increment is dropped, could a subsequent unpoison operation underflow mem->nr_hwpoison to ULONG_MAX, permanently breaking memory_block_online() with -EHWPOISON? [ ... ] > @@ -1251,4 +1257,33 @@ static unsigned long memblk_nr_poison(struct > memory_block *mem) > { > return atomic_long_read(&mem->nr_hwpoison); > } > + > +/* > + * Frames a kexec handed over are flagged as they reach the allocator, long > + * before this block exists, so memblk_nr_poison_inc() had nowhere to count > + * them. Take them from the page flag instead. > + */ > +static void memblk_nr_poison_init(struct memory_block *mem) > +{ > + unsigned long pfn = section_nr_to_pfn(mem->start_section_nr); > + unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block; > + unsigned long i, nr_poison = 0; > + > + /* A hotplugged block is created before its pages are online. */ > + if (mem->state != MEM_ONLINE) > + return; > + > + if (!range_contains_poisoned_memory(PFN_PHYS(pfn), > + nr_pages << PAGE_SHIFT)) > + return; [Severity: Medium] Does this check unconditionally return false for a fresh early MCE because the MCE is not in the inherited EFI table? If so, would this skip the manual walk, leaving the counter entirely dependent on the concurrent memory_failure() increment that might be dropped in the race window described in add_memory_block() above? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
