Re: [PATCH 06/10] x86/mm: stop marking page tables as MIX_SECTION_INFO

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The patch leaves the entire bootmem_info infrastructure as dead code without removing it.

Pre-existing issues:
- [High] Kernel page table pages and vmemmap pages are freed to the buddy allocator before their parent entries are cleared and before the TLB is flushed, creating a dangling pointer window.
--

--- Patch [6]: [PATCH 06/10] x86/mm: stop marking page tables as MIX_SECTION_INFO ---
commit 898f480b722bff5d0f4c640bb5e5d118e809c16f
Author: David Hildenbrand (Arm) <david@kernel.org>
Subject: x86/mm: stop marking page tables as MIX_SECTION_INFO

This commit stops marking boot page tables as MIX_SECTION_INFO since they
are only freed when completely empty and memory offlining doesn't benefit
from it. It updates free_pagetable() to free reserved pages directly and
empties register_page_bootmem_memmap() to reflect this change.

> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 82bb0384b8ab0..1b17dcf781e66 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -1002,21 +1002,10 @@ int arch_add_memory(int nid, u64 start, u64 size,
>  
>  static void __meminit free_pagetable(struct page *page)
>  {
> -	/* bootmem page has reserved flag */
> -	if (PageReserved(page)) {
> -#ifdef CONFIG_HAVE_BOOTMEM_INFO_NODE
> -		enum bootmem_type type = bootmem_type(page);
> -
> -		if (type == MIX_SECTION_INFO)
> -			put_page_bootmem(page);
> -		else
> -			free_reserved_page(page);
> -#else
> +	if (PageReserved(page))
>  		free_reserved_page(page);
> -#endif
> -	} else {
> +	else
>  		pagetable_free(page_ptdesc(page));
> -	}
>  }

[Severity: High]
This isn't a bug introduced by this patch, but looking at how free_pagetable()
is called during memory hot-remove:

arch/x86/mm/init_64.c:free_pte_table() {
    ...
	/* free a pte table */
	free_pagetable(pmd_page(*pmd));
	spin_lock(&init_mm.page_table_lock);
	pmd_clear(pmd);
	spin_unlock(&init_mm.page_table_lock);
    ...
}

arch/x86/mm/init_64.c:free_pmd_table() {
    ...
	/* free a pmd table */
	free_pagetable(pud_page(*pud));
	spin_lock(&init_mm.page_table_lock);
	pud_clear(pud);
	spin_unlock(&init_mm.page_table_lock);
    ...
}

Could freeing the page table pages to the buddy allocator before their parent
entries are cleared lead to a use-after-free window? If the freed page is
reallocated before the PMD or PUD is cleared, a lockless hardware MMU walker
might follow the dangling pointer and interpret the new page contents as valid
page table entries.

Would it be safer to clear the parent entries and flush the TLB before freeing
the child pages?

[ ... ]

> @@ -1579,50 +1568,6 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
>  void register_page_bootmem_memmap(unsigned long section_nr,
>  				  struct page *start_page, unsigned long nr_pages)
>  {
> -	unsigned long addr = (unsigned long)start_page;
> -	unsigned long end = (unsigned long)(start_page + nr_pages);
> -	unsigned long next;
> -	pgd_t *pgd;
> -	p4d_t *p4d;
> -	pud_t *pud;
> -	pmd_t *pmd;
> -
> -	for (; addr < end; addr = next) {
> -		pgd = pgd_offset_k(addr);
> -		if (pgd_none(*pgd)) {
> -			next = (addr + PAGE_SIZE) & PAGE_MASK;
> -			continue;
> -		}
> -		get_page_bootmem(section_nr, pgd_page(*pgd), MIX_SECTION_INFO);

[ ... ]

> -		if (!boot_cpu_has(X86_FEATURE_PSE) || !pmd_leaf(*pmd)) {
> -			next = (addr + PAGE_SIZE) & PAGE_MASK;
> -			get_page_bootmem(section_nr, pmd_page(*pmd),
> -					 MIX_SECTION_INFO);
> -		} else {
> -			next = pmd_addr_end(addr, end);
> -		}
> -	}
>  }

[Severity: Low]
By removing the implementation of register_page_bootmem_memmap(), is the rest
of the bootmem_info infrastructure left as dead code?

Without get_page_bootmem() being called here, it appears no page will ever
have its bootmem_type set to SECTION_INFO or MIX_SECTION_INFO. As a result,
the SECTION_INFO check in free_bootmem_page() would always evaluate to false,
meaning put_page_bootmem() is never executed.

Should the remaining bootmem_info code, such as
register_page_bootmem_info_node() in mm/bootmem_info.c, be removed as well to
clean up the unused infrastructure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-bootmem_info_part2-v1-0-156ce4986598@kernel.org?part=6