Re: [RFC PATCH v2 12/13] mm: shared semaphores for ANON_VMA_FRACTAL

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Lockless RCU readers access freed `anon_semaphore` memory because `anon_sema_cachep` lacks `SLAB_TYPESAFE_BY_RCU`.
- [High] Direct writers and `vm_lock` callers experience TOCTOU races and state loss due to dynamic semaphore upgrades without pointer re-verification.
- [High] An untracked `anon_node` with children retains a dangling `node->mm` pointer to an unmapped VMA's freed `mm_struct`, which KSM rmap walks dereference.
--

--- Patch [12]: [RFC PATCH v2 12/13] mm: shared semaphores for ANON_VMA_FRACTAL ---
commit 415add0f8800d6a7589361c22fc91f31e4dab44c
Author: tao <tao.wangtao@honor.com>

mm: shared semaphores for ANON_VMA_FRACTAL

This commit introduces shared semaphores for anon_node to reduce memory
overhead, reusing the root anon_node's unused parent pointer to store a
dedicated semaphore. It also adds logic to revalidate locks in rmap paths
and uses rwsem_is_write_locked to reduce contention.

> diff --git a/mm/rmap.c b/mm/rmap.c
> index 0967ba2d2a466..196af0ef07ef5 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
[ ... ]
> @@ -600,10 +662,16 @@ void __init anon_vma_init(void)
>  		.freeptr_offset = offsetof(struct anon_node, fractal_list),
>  		.ctor = anon_node_ctor,
>  	};
> +#ifdef CONFIG_ANON_VMA_SHARED_SEMS
> +	int i;
>  
> +	for (i = 0; i < ANON_NODE_RMAP_SEMS_COUNT; i++)
> +		anon_sema_init(&anon_node_root_semas[i]);
> +#endif
>  	anon_node_cachep = kmem_cache_create("anon_node",
>  			sizeof(struct anon_node), &args,
>  			SLAB_TYPESAFE_BY_RCU|SLAB_PANIC|SLAB_ACCOUNT);
> +	anon_sema_cachep = KMEM_CACHE(anon_semaphore, SLAB_PANIC|SLAB_ACCOUNT);
>  }

[Severity: High]
Can lockless RCU readers access freed memory here?

When an anon_node is destroyed, anon_root_sema_free() immediately returns
the semaphore to anon_sema_cachep. Because anon_sema_cachep is created
without SLAB_TYPESAFE_BY_RCU (unlike anon_node_cachep), the memory is
instantly available for reallocation.

If a concurrent lockless RCU reader in folio_lock_anon_vma_read() obtains a
pointer to root_anon_sema just before it is freed:

CPU1
anon_node_free()
  anon_root_sema_free()

CPU2
folio_lock_anon_vma_read()
  root_rwsem = anon_root_rmap_sem(root_anon_vma);
  if (down_read_trylock(root_rwsem)) { ... }

Could CPU2 execute down_read_trylock() on reallocated memory and cause
corruption?

[ ... ]
> @@ -758,8 +837,22 @@ static int anon_node_add_child(struct anon_node *anon_nod,
>  	struct rw_semaphore *rmap_sem = anon_node_rmap_sem(root);
>  	struct anon_node *rbc_ch = anon_nod, *next;
>  
> -	if (root->depth == 0)
> +	if (root->depth == 0) {
> +#ifdef CONFIG_ANON_VMA_SHARED_SEMS
> +		struct anon_semaphore *root_sema = anon_root_sema_alloc();
> +
> +		if (!root_sema)
> +			return -ENOMEM;
> +		down_write(rmap_sem);
> +		if (cmpxchg(&root->root_anon_sema, NULL, root_sema) != NULL)
> +			anon_root_sema_free(root_sema);
> +		root->depth = 1;
> +		up_write(rmap_sem);

[Severity: High]
Are direct writers vulnerable to a TOCTOU race when the semaphore is
upgraded?

This block upgrades the node to use a dedicated semaphore under the shared
write lock. 

If a tree modifier like unlink_anon_vmas() or vma_pre_update_rmap_base()
evaluates anon_node_rmap_sem() and gets the shared semaphore pointer, but
blocks before acquiring it, it might wake up after this upgrade completes. 

Since those paths do not appear to re-verify the pointer like
anon_node_lock_rmap_and_verify() does, will they mistakenly modify the tree
while holding the old shared lock, allowing concurrent modifications from
other threads using the new dedicated lock?

[ ... ]

[Severity: High]
Does this leave a dangling mm pointer on untracked nodes?

When a VMA is unmapped and unlink_anon_vmas() untracks the node,
anon_node_untrack_rmap_locked() skips clearing the mm pointer if the node
still has children.

If the process then exits and frees the mm_struct, could a later KSM rmap
walk pass a non-zero addr to anon_node_lookup_vma(), bypass the untracked
safeguard, and dereference the freed mm_struct in vma_lookup()?

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