Re: [PATCH 10/12] mm: enable MMU_GATHER_RCU_TABLE_FREE for sparc32

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent execution of pagetable_pte_ctor() and pagetable_dtor() on shared backing pages corrupts folio state.
--

commit f7114a4ac52b3be28e86c3e8ab381854e55bfcf6
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>

mm: enable MMU_GATHER_RCU_TABLE_FREE for sparc32
    
This commit enables MMU_GATHER_RCU_TABLE_FREE for sparc32 by using an RCU
callback to free page tables. It introduces __tlb_remove_table() and modifies
__pte_free() to handle NULL mm pointers when called from softirq context.

> But more importantly - the reference count increment in pte_alloc_one() and
> decrement in __pte_free() are atomic with full ordering, so it simply isn't
> possible for there to be a meaningful race here.

[Severity: High]
Does this actually prevent concurrent execution of the page table constructor
and destructor?

The atomic refcount ensures the reference count itself is safe, but it does
not serialize the execution of the constructor and destructor bodies.

If a process unmaps memory and queues PTE pages for RCU free, a softirq could
begin executing __pte_free() and drop the page refcount to 1, entering
pagetable_dtor().

If a process concurrently page faults and allocates a new PTE table in the same
4KB physical page, it increments the refcount to 2 and enters
pagetable_pte_ctor(). 

Could both threads execute the constructor and destructor concurrently,
corrupting the folio state?

> diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c
> index 9a74902ad1814..2a2c7bd210114 100644
> --- a/arch/sparc/mm/srmmu.c
> +++ b/arch/sparc/mm/srmmu.c
[ ... ]
> @@ -359,19 +359,39 @@ pgtable_t pte_alloc_one(struct mm_struct *mm)
>  	return ptep;
>  }
>  
> -void pte_free(struct mm_struct *mm, pgtable_t ptep)
> +static void __pte_free(struct mm_struct *mm, pgtable_t ptep)
>  {
> +	const bool process_context = mm;
>  	struct page *page;
>  
>  	page = pfn_to_page(__nocache_pa((unsigned long)ptep) >> PAGE_SHIFT);
> -	spin_lock(&mm->page_table_lock);
> +	if (process_context)
> +		spin_lock(&mm->page_table_lock);
>  	if (page_ref_dec_return(page) == 1)
>  		pagetable_dtor(page_ptdesc(page));
        ^^^^

[Severity: High]
Can this page table destructor run concurrently with the page table
constructor in pte_alloc_one()?

Since process_context is false when called from softirq context via RCU, this
executes without a lock protecting the physical page.

If pte_alloc_one() concurrently allocates another PTE table in the same
backing page, increments the refcount to 2, and calls
pagetable_pte_ctor(mm, page_ptdesc(page)), could this lead to non-atomic
modifications to folio->page.page_type?

Concurrent execution of __folio_clear_pgtable() in the destructor and
initialization in the constructor could corrupt the page state, which may
lead to kernel crashes when pages are returned to the buddy allocator.

> -	spin_unlock(&mm->page_table_lock);
> +	if (process_context)
> +		spin_unlock(&mm->page_table_lock);
>  
>  	srmmu_free_nocache(ptep, SRMMU_PTE_TABLE_SIZE);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-rcu-pagetable-freeing-v1-0-5456a81c8212@kernel.org?part=10