Re: [PATCH v6 4/7] mm/memblock: add a reserved-region flag for poisoned frames

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: page-level sharing detection fails due to unaligned base check
--

commit 91f3abf246b482e4b3339361ad3572545e8e83f6
Author: Breno Leitao <leitao@debian.org>

mm/memblock: add a reserved-region flag for poisoned frames

This commit adds the MEMBLOCK_RSRV_HWPOISON flag to memblock to reserve
hardware-poisoned frames found by an earlier kernel. It introduces
memblock_reserve_hwpoison() to handle these reservations while attempting
to avoid overlap with existing reserved regions. Additionally, it ensures
these frames are flagged with PG_hwpoison instead of PG_reserved during
initialization.

> diff --git a/mm/memblock.c b/mm/memblock.c
> index 31961ed628b0a..d20d99972a4db 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c

[ ... ]

> @@ -1015,6 +1018,55 @@ int __init_memblock __memblock_reserve(phys_addr_t base, phys_addr_t size,
>  	return memblock_add_range(&memblock.reserved, base, size, nid, flags);
>  }
>  
> +int __init memblock_reserve_hwpoison(phys_addr_t base, phys_addr_t size)
> +{
> +	phys_addr_t end = base + size;
> +	bool reserved = false;
> +	int err = 0;
> +
> +	while (base < end) {
> +		phys_addr_t next = end;
> +		struct memblock_region *r;
> +		bool covered = false;
> +
> +		for_each_reserved_mem_region(r) {
> +			if (r->base + r->size <= base)
> +				continue;
> +			if (r->base >= end)
> +				break;
> +			if (PAGE_ALIGN_DOWN(r->base) <= base) {

[Severity: High]
Does this byte-level check cause the loop to skip regions that share
the same page but end before the new reservation's base?

The commit message states that a page shared with an earlier reservation
stays with it, so no page is flagged twice. However, if an existing
reservation spans 0x1000-0x1100 and the new HWPOISON reservation requests
0x1800-0x1900 (both in the same 4KB page), 0x1100 <= 0x1800 is true.
This causes the loop to skip the region and bypass the subsequent
PAGE_ALIGN_DOWN overlap check, leading to the new region being reserved
alongside the old one.

If the prior reservation was a KERNEL reservation, the page erroneously
receives both PG_reserved and PG_hwpoison.

[ ... ]

> +/*
> + * Frames an earlier kernel found poisoned end up as a frame poisoned by this
> + * kernel would: PG_hwpoison, not PG_reserved. Holes in the range stay reserved.
> + */
> +static void __init memmap_init_hwpoison_range(phys_addr_t start,
> +					      phys_addr_t end, int nid)
> +{
> +	unsigned long pfn;
> +
> +	for_each_valid_pfn(pfn, PFN_DOWN(start), PFN_UP(end)) {
> +		struct page *page = pfn_to_page(pfn);
> +
> +		init_deferred_page(pfn, nid);
> +
> +		if (memblock_is_map_memory(PFN_PHYS(pfn)))
> +			hwpoison_boot_page(page);

[Severity: High]
Can this path execute multiple times for the same PFN when a page contains
multiple unmerged HWPOISON fragments?

Because of the unaligned base check bypass above,
memmap_init_reserved_pages() will process the same page twice. Since
hwpoison_boot_page() lacks a TestSetPageHWPoison() guard, it unconditionally
increments num_poisoned_pages and calls adjust_managed_page_count(page, 1)
on every invocation.

This artificially inflates both the global poisoned page count and the
zone's managed page count, potentially leading to page allocator
underflows and corrupted VM statistics.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-hwpoison-kho-v6-0-f304255147da@debian.org?part=4