Re: [RFC PATCH v2 08/13] mm: implement anonymous folio rmap for ANON_VMA_FRACTAL

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Accessing a dangling `mm_struct` pointer in `anon_node_lookup_vma` causes Use-After-Free (UAF) on the `mm_struct` or its maple tree.
- [High] Lockless access to a `SLAB_TYPESAFE_BY_RCU` VMA after RCU is dropped in `anon_node_lookup_vma` causes Use-After-Free.
- [Medium] ANON_RMAP_FOREACH_VMA macro causes an infinite loop if the caller uses a `continue` statement.
- [Medium] Using 0 as a sentinel address breaks legitimate VMAs mapped at address 0x0.
--

--- Patch [8]: [RFC PATCH v2 08/13] mm: implement anonymous folio rmap for ANON_VMA_FRACTAL ---
commit 6798bdd36311c3a484df289386e32cf89ecdeba9
Author: tao <tao.wangtao@honor.com>

mm: implement anonymous folio rmap for ANON_VMA_FRACTAL

This commit adds the ANON_RMAP_FOREACH_VMA() macro to encapsulate rmap
traversal. It traverses the folio's own anon_node and its descendants, using
VMA updates to advance the page offset. For KSM, the VMA is located directly
using the supplied address.

> diff --git a/mm/internal.h b/mm/internal.h
> index 12473ac84fa4e..5f96b15246bf9 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h

[ ... ]

> @@ -303,6 +303,55 @@ static inline unsigned long anon_node_rmap_count(struct anon_node *anon_nod)
>  	return atomic_long_read(&anon_nod->rbc) & ANON_RMAP_BASE_COUNT_MAX;
>  }
>  
> +static inline unsigned long anon_node_rmap_address(struct anon_node *anon_nod,
> +		unsigned long pgoff)
> +{
> +	if (!anon_node_rmap_count(anon_nod))
> +		return 0;
> +	return anon_node_rmap_base(anon_nod) + (pgoff << PAGE_SHIFT);
> +}
> +
> +static inline struct vm_area_struct *anon_node_lookup_vma(
> +	struct anon_node *anon_nod, unsigned long addr, unsigned long pgoff)
> +{
> +	struct vm_area_struct *vma;
> +
> +	if (!addr)
> +		addr = anon_node_rmap_address(anon_nod, pgoff);
> +	if (!anon_nod->mm || !addr)

[Severity: Medium]
Will this silently ignore valid mappings at address 0x0?

If mmap_min_addr is configured to 0, it seems a valid anonymous VMA could be
explicitly mapped at virtual address 0x0. If this happens, addr will be 0, and
this check will return NULL, causing the valid VMA to be hidden from rmap walks.

> +		return NULL;
> +
> +	vma = vma_lookup(anon_nod->mm, addr);

[Severity: High]
Could anon_nod->mm be a dangling pointer here?

When a VMA is unmapped, if unlink_anon_nodes() skips clearing the mm pointer
for the root anon_node, and the anon_node remains alive because folios are in
the swap cache or pinned by GUP, its mm pointer might dangle after the process
exits and its mm_struct is freed.

If ANON_RMAP_FOREACH_VMA() is called later during reclaim, will it dereference
the freed mm_struct here when calling vma_lookup()?

> +	return (vma && vma->anon_vma == (void *)anon_nod) ? vma : NULL;

[Severity: High]
Is it safe to access vma->anon_vma without RCU protection?

vma_lookup() delegates to mtree_load(), which acquires and then releases the RCU
read lock internally. Since the caller of anon_node_lookup_vma() doesn't hold
the mmap_lock (only the anon_node lock), and no outer RCU critical section is
held, the returned VMA pointer appears to be unprotected.

Because the VMA cache is SLAB_TYPESAFE_BY_RCU, its backing memory can be
unmapped and freed by a concurrent unmap outside of an RCU critical section.
Will dereferencing vma->anon_vma here result in a use-after-free?

> +}
> +
> +void vm_lock_anon_node(struct mm_struct *mm, struct anon_node *anon_nod);
> +void vm_unlock_anon_node(struct anon_node *anon_nod);
> +int anon_node_trylock_rmap(struct anon_node *anon_nod);
> +void anon_node_lock_rmap(struct anon_node *anon_nod);
> +void anon_node_unlock_rmap(struct anon_node *anon_nod);
> +
> +#define ANON_RMAP_FOREACH_VMA(anon_vma, addr, start, last, rmap_proc_vma) \
> +do { \
> +	struct vm_area_struct *vma = NULL; /* must be named vma */ \
> +	struct anon_node *_anon_nod = (void *)(anon_vma); \
> +	struct anon_node *_nod = _anon_nod; \
> +	unsigned long _pgoff = (start); \
> +	unsigned long _nr = 0, _total = 1; \
> +	\
> +	while (_nod) { \
> +		vma = anon_node_lookup_vma(_nod, (addr), _pgoff); \
> +		if (vma) \
> +			(rmap_proc_vma); \

[Severity: Medium]
What happens if the caller provides a block for rmap_proc_vma that contains a
continue statement?

Because the loop advancement logic:

		_pgoff = vma ? vma->vm_pgoff + vma_pages(vma) : _pgoff + 1;
		...
		_nod = anon_node_next_descendant(_anon_nod, _nod);

is located after the execution of rmap_proc_vma, a continue statement inside the
caller's block would jump straight to the while (_nod) condition.

Will this skip the advancement of _nod and _pgoff, causing the macro to infinite
loop on the same node?

> +		_pgoff = vma ? vma->vm_pgoff + vma_pages(vma) : _pgoff + 1; \
> +		if (!(addr) && _pgoff <= (last)) \
> +			continue; \
> +		_pgoff = (start); \
> +		BUG_ON(++_nr > (_total += _nod->nr_children)); \
> +		_nod = anon_node_next_descendant(_anon_nod, _nod); \
> +	} \
> +} while (0)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707063308.29176-1-tao.wangtao@honor.com?part=8