Re: [PATCH v2 07/12] mm: hugetlb_vmemmap: simplify shared tail page handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: hugetlb_vmemmap: uninitialized tail page read in vmemmap_optimize_pte()
--

commit 32511c4a4d8ec3d45e0f629f89666a78dc9372ee
Author: Muchun Song <muchun.song@linux.dev>

mm: hugetlb_vmemmap: simplify shared tail page handling

This commit removes the vmemmap_tail field from struct vmemmap_remap_walk and
updates vmemmap_optimize_pte() and vmemmap_restore_pte() to derive the shared
tail page directly from the folio being processed.

> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index 5e84004db4b87..d0af251aa533c 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c

[ ... ]

> @@ -213,53 +211,53 @@ static struct page *vmemmap_dup_page_from_list(const struct page *src,
>  	return dst;
>  }
>  
> -static void vmemmap_optimize_pte(pte_t *pte, unsigned long addr,
> -				 struct vmemmap_remap_walk *walk)
> +static inline struct page *folio_vmemmap_shared_tail_page(const struct folio *folio)
>  {
> -	struct page *page = pte_page(ptep_get(pte));
> -	pte_t entry;
> -
> -	/* Remapping the head page requires r/w */
> -	if (unlikely(walk->nr_walked == 0)) {
> -		struct page *head;
> +	return vmemmap_shared_tail_page(folio_order(folio), folio_zone(folio));
> +}
>  
> -		VM_WARN_ON_ONCE(!PageHead((const struct page *)addr));
> +static int vmemmap_optimize_pte(pte_t *pte, unsigned long addr,
> +				struct vmemmap_remap_walk *walk)
> +{
> +	const struct page *page = (const struct page *)addr;
> +	const struct folio *folio = page_folio(page);
> +	struct page *src = pte_page(ptep_get(pte)), *dst;
>  
> -		head = vmemmap_dup_page_from_list(page, walk->vmemmap_pages);
> -		entry = mk_pte(head, PAGE_KERNEL);
> +	if (likely(vmemmap_optimizable_pfn(page_to_pfn(page), folio_order(folio)))) {
> +		dst = folio_vmemmap_shared_tail_page(folio);
> +		if (!dst)
> +			return -ENOMEM;
>  	} else {
> -		VM_WARN_ON_ONCE(!PageTail((const struct page *)addr));
> -
> -		/*
> -		 * Map the shared tail page writable. A lockless get that read
> -		 * a non-zero refcount before the remap can still do its
> -		 * cmpxchg after it. The refcount is zero, so the cmpxchg
> -		 * fails and leaves the page unchanged.
> -		 */
> -		entry = mk_pte(walk->vmemmap_tail, PAGE_KERNEL);
> +		dst = vmemmap_dup_page_from_list(src, walk->vmemmap_pages);
>  	}

[Severity: High]
Can calling page_folio(page), folio_order(folio), and
folio_vmemmap_shared_tail_page(folio) on every PTE in vmemmap_optimize_pte()
read uninitialized tail struct pages for bootmem gigantic hugepages?

During early boot in alloc_bootmem_huge_page(), gigantic hugepages mark all
tail pages [pfn + 1, pfn + nr_pages) with memblock_reserved_mark_noinit(), so
memmap_init_reserved_pages() skips initializing their struct pages:

mm/hugetlb.c:alloc_bootmem_huge_page() {
    ...
	if (hugetlb_vmemmap_optimizable(h))
		hugetlb_vmemmap_optimize_bootmem_page(pfn, huge_page_order(h));
    ...
	memblock_reserved_mark_noinit(PFN_PHYS(pfn + 1),
				      (nr_pages - 1) * PAGE_SIZE);
}

When a bootmem gigantic hugepage is not aligned to PAGES_PER_SECTION (for
example, when nr_pages < PAGES_PER_SECTION, such as on 64-bit LoongArch with
16KB base pages where SECTION_SIZE_BITS is 29 and PMD_SHIFT is 25 with
CONFIG_ARCH_FORCE_MAX_ORDER=10), section_set_compound_order_range() returns
early without setting section->compound_page_order:

include/linux/vmemmap-optimization.h:section_set_compound_order_range() {
	unsigned long section_nr = pfn_to_section_nr(pfn);

	if (!IS_ALIGNED(pfn | nr_pages, PAGES_PER_SECTION))
		return;
    ...
}

In that case, sparse_init() allocates regular vmemmap pages via
memmap_alloc(), leaving them uninitialized (or filled with 0xff under
CONFIG_DEBUG_VM).

Later, gather_bootmem_prealloc_node() initializes only the first
VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES struct pages (covering only the first
vmemmap PTE), and prep_and_add_bootmem_folios() defers initializing the
remaining tail struct pages until after hugetlb_vmemmap_optimize_folios()
runs and fails:

mm/hugetlb.c:gather_bootmem_prealloc_node() {
    ...
		hugetlb_folio_init_vmemmap(folio, h,
					   VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES);
		init_new_hugetlb_folio(folio);

		if (vmemmap_optimizable_order(pfn_to_section_compound_order(pfn)))
			folio_set_hugetlb_vmemmap_optimized(folio);
    ...
}

mm/hugetlb.c:prep_and_add_bootmem_folios() {
	/* Send list for bulk vmemmap optimization processing */
	hugetlb_vmemmap_optimize_folios(h, folio_list);

	list_for_each_entry_safe(folio, tmp_f, folio_list, lru) {
		if (!folio_test_hugetlb_vmemmap_optimized(folio)) {
			hugetlb_folio_init_tail_vmemmap(folio, h,
					VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES,
					pages_per_huge_page(h));
		}
    ...
}

Previously, __hugetlb_vmemmap_optimize_folio() derived vmemmap_tail upfront
from the initialized head folio, and vmemmap_optimize_pte() checked
walk->nr_walked == 0 without inspecting tail struct pages.

With this change, on the second and subsequent PTEs (where addr >=
vmemmap_start + VMEMMAP_OPTIMIZATION_SIZE), page->compound_info in the tail
struct page at addr has not been initialized by prep_compound_tail() yet:

1. If the raw vmemmap backing pages are zeroed, page_folio(page) returns page
itself rather than the head folio, folio_order(folio) returns 0, and
vmemmap_optimizable_pfn() returns false. vmemmap_optimize_pte() then falls
back to vmemmap_dup_page_from_list(src, walk->vmemmap_pages) for every tail
PTE, rotating the displaced vmemmap pages across the tail PTEs instead of
mapping the shared tail page, while leaving HPG_vmemmap_optimized set so
prep_and_add_bootmem_folios() skips hugetlb_folio_init_tail_vmemmap().

2. If CONFIG_DEBUG_VM is enabled, page_init_poison() fills the uninitialized
tail struct pages with 0xff. _compound_head(page) then evaluates to the
poisoned tail page itself, and folio_order(folio) reads 0xff, leading to an
out-of-bounds shift in vmemmap_optimizable_pfn() and an out-of-bounds
node_zones access in folio_zone(folio).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102032525.3597743-1-songmuchun@bytedance.com?part=7