Re: [RFC PATCH] riscv: mm: allocate split linear mapping page tables via the standard page table allocation APIs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Low] riscv: mm: alloc_kernel_pgtable() duplicates pmd_alloc_one() and
  pud_alloc_one()
- [Low] riscv: mm: inaccurate comment and changelog on freeing linear
  map page tables
- [Low] riscv: mm: missing Fixes trailer for 311cd2f6e253 and RFC text
  in changelog
--

commit 1708f72f80d7427a70293de6013d47c497785abe
Author: li.hanting@zte.com.cn <li.hanting@zte.com.cn>

> riscv: mm: allocate split linear mapping page tables via the standard page table allocation APIs
>
> Hi,
>
> Commit 9e4a3ec3411b ("x86/mm/pat: Allocate split page tables as kernel
> page tables") recently fixed a latent inconsistency in the x86 CPA code
> (assigned CVE-2026-97525): __split_large_page() allocated the new PTE
> page with a bare pagetable_alloc(), bypassing pte_alloc_one_kernel(), so
> the page table constructor never ran and the resulting page was neither
> flagged PG_table, nor accounted in NR_PAGETABLE, nor marked as a kernel
> page table.
>
> While looking at the fallout of that commit, I noticed that RISC-V
> open-codes the same pattern when splitting the linear mapping on behalf
> of set_memory_*(): __split_linear_mapping_{pmd,pud,p4d}() allocate the
> new page table pages with bare alloc_page(GFP_KERNEL) - three call
> sites in arch/riscv/mm/pageattr.c, introduced by 311cd2f6e253 ("riscv:
> Fix set_memory_XX() and set_direct_map_XX() by splitting huge linear
> mappings").
>
> The consequences are the same as on pre-fix x86: page tables that live
> in init_mm without the PG_table flag, without NR_PAGETABLE accounting
> and without the kernel ptdesc marking.
>
> To be clear about severity: this is a latent inconsistency, not an
> exploitable bug today. RISC-V never collapses the split linear mapping
> back, and these page tables are never freed, so there is no free-path
> mismatch of the kind the x86 fix was concerned about. But the
> accounting mismatch is real (/proc/vmstat under-counts kernel page
> tables), and any future work that frees these tables - linear map
> memory hotplug, collapse support or anything similar - would inherit
> the exact bug class the x86 fix addressed.
>
> This RFC aligns the three sites with the standard allocation paths:
>
>  - the PTE level switches to pte_alloc_one_kernel(), exactly like the
>    x86 fix;
>
>  - the PMD and PUD levels have no *_alloc_one_kernel() variant today,
>    so a small local helper wraps pagetable_alloc() +
>    __pagetable_ctor() + ptdesc_set_kernel(), mirroring what the
>    generic PTE path does. GFP_PGTABLE_KERNEL also brings in
>    __GFP_ZERO, which the open-coded allocation was missing.
>
> No functional change is intended; the observable difference is that
> split linear mapping page tables now show up in
> /proc/vmstat:nr_page_table_pages.
>
> Open questions:
>
>  1. Would it make sense to provide generic pmd_alloc_one_kernel() /
>     pud_alloc_one_kernel() variants in asm-generic/pgalloc.h so that
>     architectures do not have to open-code this dance? This is the
>     main reason this is an RFC rather than a regular patch.
>
>  2. Since these tables are never freed today, is it preferable to take
>     this alignment now (my preference: it removes a trap for any
>     future free-path work), or defer it until such work actually
>     lands?
>
>     (Note: the generic CPA rework series "[PATCH RFC 00/18] arch, mm:
>     create a generic set_memory/change_page_attr core" would eventually
>     replace this code entirely; this patch is intended as a small fix in
>     the meantime. The series author has said he is waiting for such
>     fixes to settle before posting a non-RFC version.)
>
> Comments welcome.
>
> Signed-off-by: Hanting Li <li.hanting@zte.com.cn>

[Severity: Low]
Should this include a Fixes: tag for the commit that introduced the open-coded
alloc_page(GFP_KERNEL) calls:

  Fixes: 311cd2f6e253 ("riscv: Fix set_memory_XX() and set_direct_map_XX() by splitting huge linear mappings")

Also, the conversational notes ("Hi,", "Open questions:", "Comments welcome.")
belong below the "---" tear line so they do not end up in the git changelog.

> diff --git a/arch/riscv/mm/pageattr.c b/arch/riscv/mm/pageattr.c
> index 5b3cf326455db..f5608414b6fc0 100644
> --- a/arch/riscv/mm/pageattr.c
> +++ b/arch/riscv/mm/pageattr.c

[ ... ]

> @@ -93,6 +95,26 @@ static const struct mm_walk_ops pageattr_ops = {
>  };
>  
>  #ifdef CONFIG_64BIT
> +/*
> + * Kernel page tables allocated for the split linear mapping are never
> + * freed today, but they must still go through the page table constructor
> + * so that they are flagged PG_table, accounted in NR_PAGETABLE and marked
> + * as kernel page tables, like any other kernel page table allocated via
> + * the standard allocation APIs.
> + */

[Severity: Low]
Is this comment (and the corresponding statement in the commit message)
accurate that split linear mapping page tables are never freed today?

Under CONFIG_MEMORY_HOTPLUG, arch_remove_memory() -> remove_linear_mapping()
walks the linear mapping and frees empty non-reserved PTE, PMD, and PUD page
tables via free_pte_table(), free_pmd_table(), and free_pud_table() ->
free_pgtable_page():

arch/riscv/mm/init.c:free_pgtable_page() {
	if (PageReserved(page))
		free_reserved_page(page);
	else
		pagetable_free(page_ptdesc(page));
}

If hotplugged memory has its linear mapping split via set_memory_*() or
set_direct_map_*() and is later hot-removed, those split page tables are freed
through pagetable_free(), which checks PageTable() and ptdesc_test_kernel().

> +static struct ptdesc *alloc_kernel_pgtable(void)
> +{
> +	struct ptdesc *ptdesc = pagetable_alloc(GFP_PGTABLE_KERNEL, 0);
> +
> +	if (!ptdesc)
> +		return NULL;
> +
> +	__pagetable_ctor(ptdesc);
> +	ptdesc_set_kernel(ptdesc);
> +
> +	return ptdesc;
> +}

[ ... ]

> @@ -149,24 +170,25 @@ static int __split_linear_mapping_pud(p4d_t *p4dp,
>  			continue;
>  
>  		if (pud_leaf(pudp_get(pudp))) {
> -			struct page *pmd_page;
> +			struct ptdesc *ptdesc;
>  			unsigned long pfn = _pud_pfn(pudp_get(pudp));
>  			pgprot_t prot = __pgprot(pud_val(pudp_get(pudp)) & ~_PAGE_PFN_MASK);
>  			pmd_t *pmdp_new;
>  			int i;
>  
> -			pmd_page = alloc_page(GFP_KERNEL);
> -			if (!pmd_page)
> +			ptdesc = alloc_kernel_pgtable();
> +			if (!ptdesc)
>  				return -ENOMEM;
>  
> -			pmdp_new = (pmd_t *)page_address(pmd_page);
> -			for (i = 0; i < PTRS_PER_PMD; ++i, ++pmdp_new)
> -				set_pmd(pmdp_new,
> +			pmdp_new = (pmd_t *)ptdesc_address(ptdesc);

[ ... ]

> @@ -198,23 +220,23 @@ static int __split_linear_mapping_p4d(pgd_t *pgdp,
>  			continue;
>  
>  		if (p4d_leaf(p4dp_get(p4dp))) {
> -			struct page *pud_page;
> +			struct ptdesc *ptdesc;
>  			unsigned long pfn = _p4d_pfn(p4dp_get(p4dp));
>  			pgprot_t prot = __pgprot(p4d_val(p4dp_get(p4dp)) & ~_PAGE_PFN_MASK);
>  			pud_t *pudp_new;
>  			int i;
>  
> -			pud_page = alloc_page(GFP_KERNEL);
> -			if (!pud_page)
> +			ptdesc = alloc_kernel_pgtable();
> +			if (!ptdesc)
>  				return -ENOMEM;
>  
>  			/*
>  			 * Fill the pud level with leaf puds that have the same
>  			 * protections as the leaf p4d.
>  			 */
> -			pudp_new = (pud_t *)page_address(pud_page);
> -			for (i = 0; i < PTRS_PER_PUD; ++i, ++pudp_new)
> -				set_pud(pudp_new,
> +			pudp_new = (pud_t *)ptdesc_address(ptdesc);

[Severity: Low]
Can __split_linear_mapping_pud() and __split_linear_mapping_p4d() use the
existing generic pmd_alloc_one(&init_mm, vaddr) and pud_alloc_one(&init_mm,
vaddr) helpers instead of introducing alloc_kernel_pgtable()?

Unlike pte_alloc_one(), which returns pgtable_t and unconditionally uses
GFP_PGTABLE_USER, the generic pmd_alloc_one() and pud_alloc_one() in
include/asm-generic/pgalloc.h already check for mm == &init_mm, select
GFP_PGTABLE_KERNEL, run the level-specific constructors pagetable_pmd_ctor()
and pagetable_pud_ctor() along with pmd_init() and pud_init(), call
ptdesc_set_kernel(), and return pmd_t * and pud_t * directly.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/202611091101.6A9B1Nf3015337@mse-db.zte.com.cn?part=1