Re: [PATCH v5 7/9] drivers/base/memory: count inherited poisoned frames into the block

[sashiko-bot@kernel.org]

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 <leitao@debian.org>

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/20260915-hwpoison-kho-v5-0-3bc7a57bd503@debian.org?part=7