Re: [PATCH v5 9/9] mm/memory-failure: keep inherited poisoned frames out of the buddy allocator

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: panic when accepting clean pages sharing an unaccepted unit with poisoned pages
- [Medium] mm: lost memory failure stats due to lockless update in hwpoison_boot_page()
--

commit 92cea9db628cbe8f9651b225628684a39c80ddd4
Author: Breno Leitao <leitao@xxxxxxxxxx>
Subject: mm/memory-failure: keep inherited poisoned frames out of the buddy allocator

This commit intercepts inherited poisoned pages before they enter the buddy
allocator. It processes memory blocks by checking for poisoned pages, flagging
them, and passing the remaining clean pages through the allocator. This
prevents unaccepted memory blocks with poisoned frames from being mistakenly
re-merged.

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index b07b5f4751cb9..cca67a2702a42 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
[ ... ]
> @@ -1597,6 +1597,25 @@ static void __meminit accept_and_free_block(struct page *page,
>  	__free_pages_ok(page, order, FPI_TO_TAIL);
>  }
>  
> +static void __meminit free_poisoned_block(struct page *page, unsigned int order,
> +					  enum meminit_context context)
> +{
> +	unsigned long i, nr_pages = 1UL << order;
> +
> +	for (i = 0; i < nr_pages; i++) {
> +		struct page *p = page + i;
> +		phys_addr_t phys = page_to_phys(p);
> +
> +		if (range_contains_poisoned_memory(phys, PAGE_SIZE)) {
> +			hwpoison_boot_page(p, context);

[Severity: Medium]
Is this call to hwpoison_boot_page() safe from data races during memory
hotplug?

When called during hotplug (where context == MEMINIT_HOTPLUG),
hwpoison_boot_page() calls update_per_node_mf_stats() which performs
non-atomic read-modify-write operations on the node stats:

mm/memory-failure.c:update_per_node_mf_stats() {
    ...
    ++mf_stats->recovered;
    ...
}

Since hwpoison_boot_page() does not acquire mf_mutex, could a concurrent
memory_failure() execution on the same NUMA node corrupt the recovered and
total counters?

> +			continue;
> +		}
> +
> +		/* this part of the block is not poisoned */
> +		accept_and_free_block(p, 0);

[Severity: High]
Can this sequence cause a kernel panic when accepting clean pages that share
an unaccepted memory unit with poisoned pages?

When a clean page belongs to an unaccepted memory region,
accept_and_free_block() invokes accept_memory(). However, accept_memory()
operates at the unit_size granularity (typically 2MB) and rounds down the
start address to this boundary before calling arch_accept_memory():

drivers/firmware/efi/unaccepted_memory.c:accept_memory() {
    ...
    arch_accept_memory(phys_start, phys_end);
    ...
}

Because the clean page and the poisoned page share the same unaccepted
unit_size block, won't arch_accept_memory() be inadvertently executed on the
poisoned page as well?

If arch_accept_memory() attempts to accept physically poisoned memory (such
as via TDX TDG.MEM.PAGE.ACCEPT), could this trigger an unrecoverable Machine
Check Exception?

> +	}
> +}
> +
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-hwpoison-kho-v5-0-3bc7a57bd503@xxxxxxxxxx?part=9