Re: [PATCH v2 05/15] mm: extend pte batch and leaf-size helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The new `__ptep_leaf_size` API encourages a double-read TOCTOU race in the lockless page table walker `perf_get_pgtable_size()`, violating generic lockless walk concurrency rules.
- [Low] The `__pte_batch_clear_ignored()` inline function breaks the build for architectures without an MMU (!CONFIG_MMU).
--

--- Patch [5]: [PATCH v2 05/15] mm: extend pte batch and leaf-size helpers ---
commit a6d281b7dea89e7c1eed5117dbe4f8bc7a9fd3f6
Author: Yunhui Cui <cuiyunhui@bytedance.com>

mm: extend pte batch and leaf-size helpers

This commit extends pte_batch_hint() with batch flags and introduces
__ptep_leaf_size() to give leaf-size users access to the original PTE pointer.
It also moves batch flag helpers into pgtable.h.

> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 8c093c119e5a8..da14328093a86 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h

[ ... ]

> @@ -1917,6 +1948,17 @@ static inline pmd_t pmd_swp_clear_soft_dirty(pmd_t pmd)
>  }
>  #endif
>  
> +static inline pte_t __pte_batch_clear_ignored(pte_t pte, fpb_t flags)
> +{
> +	if (!(flags & FPB_RESPECT_DIRTY))
> +		pte = pte_mkclean(pte);
> +	if (likely(!(flags & FPB_RESPECT_SOFT_DIRTY)))
> +		pte = pte_clear_soft_dirty(pte);
> +	if (likely(!(flags & FPB_RESPECT_WRITE)))
> +		pte = pte_wrprotect(pte);
> +	return pte_mkold(pte);
> +}
> +

[Severity: Low]
Does moving __pte_batch_clear_ignored() to the global scope in pgtable.h
break the build for !CONFIG_MMU architectures?

The functions pte_mkclean(), pte_wrprotect(), and pte_mkold() are generally
undefined without an MMU. Since this is now placed outside of any CONFIG_MMU
guard, it appears it could cause implicit function declaration errors for
nommu targets.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 51c1200ea3fdd..37b860abe9e61 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -8551,7 +8551,7 @@ static u64 perf_get_pgtable_size(struct mm_struct *mm, unsigned long addr)
>  
>  	pte = ptep_get_lockless(ptep);
>  	if (pte_present(pte))
> -		size = __pte_leaf_size(pmd, pte);
> +		size = __ptep_leaf_size(pmd, ptep, pte);
>  	pte_unmap(ptep);
>  #endif /* CONFIG_HAVE_GUP_FAST */
>  

[Severity: Medium]
Could the new __ptep_leaf_size() API introduce a Time-Of-Check to
Time-Of-Use race in lockless walkers like perf_get_pgtable_size()?

In perf_get_pgtable_size(), we acquire an atomic snapshot of the PTE using
ptep_get_lockless(), and then pass both the snapshot (pte) and the raw
pointer (ptep) to __ptep_leaf_size().

If an architecture implementation of __ptep_leaf_size() reads the ptep
pointer directly to handle folded mappings, it would perform a second
non-atomic read. A concurrent page table modification could alter the PTE
between the initial ptep_get_lockless() snapshot and the architecture's
read, leading to a mismatched evaluation.

Lockless walkers typically need to derive all information from a single
atomic snapshot to ensure concurrency rules are respected.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784201104.git.cuiyunhui@bytedance.com?part=5