Re: [PATCH 10/15] mm: defer anon_vma creation with ANON_VMA_LAZY

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional increment of the root anon_vma refcount when reusing an existing anon_vma causes a memory leak.
--

commit e7b82bf7c91f9ff13c87c5b6d44b365dcfcf93c5
Author: tao <tao.wangtao@honor.com>

mm: defer anon_vma creation with ANON_VMA_LAZY

This commit defers anon_vma creation until fork by marking VMAs as
ANON_VMA_LAZY to reduce allocation overhead. During fork, lazy VMAs are
upgraded to establish the sharing topology, and child VMAs are created
without allocating a new anon_vma.

> diff --git a/mm/rmap.c b/mm/rmap.c
> index 12693c6897120..59848e1aaf531 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c

[ ... ]

> @@ -282,19 +393,30 @@ int __anon_vma_prepare(struct vm_area_struct *vma)
>  
>  	anon_vma = find_mergeable_anon_vma(vma);
>  	allocated = NULL;
> -	if (!anon_vma) {
> +	/* If parent_anon_vma exists, mergeable anon_vma root must match it. */
> +	if (!anon_vma ||
> +		(parent_anon_vma && anon_vma->root != parent_anon_vma->root)) {
>  		anon_vma = anon_vma_alloc();
>  		if (unlikely(!anon_vma))
>  			goto out_enomem_free_avc;
> -		anon_vma->num_children++; /* self-parent link for new root */
>  		allocated = anon_vma;
> +		if (parent_anon_vma) {
> +			anon_vma->root = parent_anon_vma->root;
> +			anon_vma->parent = parent_anon_vma;
> +		}
>  	}
>  
>  	anon_tree = make_anon_vma_tree(anon_vma);
> +	if (upgrade_lazy)
> +		is_lazy = vma_unlink_anon_vma_lazy(vma, anon_tree);
>  	anon_vma_tree_lock_write(anon_tree);
>  	/* page_table_lock to protect against threads */
>  	spin_lock(&mm->page_table_lock);
> -	if (likely(!vma->anon_vma)) {
> +	if (likely(!vma->anon_vma || is_lazy)) {
> +		if (anon_vma->root != anon_vma)
> +			get_anon_vma(anon_vma->root);

[Severity: High]
Does this code leak the root anon_vma when reusing an existing anon_vma?

When find_mergeable_anon_vma() finds a compatible anon_vma to reuse,
allocated is left as NULL. However, the code proceeds to unconditionally
call get_anon_vma(anon_vma->root) as long as anon_vma->root != anon_vma.

The root's refcount tracks the number of child anon_vma structs, not the
number of VMAs sharing the child anon_vma. Because a new anon_vma was not
allocated, the root's refcount was already correctly incremented when the
existing child anon_vma was originally created.

When the VMAs are later unmapped and the child anon_vma's interval tree
becomes completely empty, __put_anon_vma() is called exactly once for the
child anon_vma, which decrements the root's refcount exactly once. Since the
root's refcount is incremented every time a VMA reuses the child anon_vma
but only decremented once when the child is freed, it seems the extraneous
references will permanently leak root anon_vma structures.

> +		if (allocated)
> +			anon_vma->parent->num_children++;
>  		vma->anon_vma = anon_tree;
>  		anon_vma_chain_assign(vma, avc, anon_vma);
>  		anon_vma_interval_tree_insert(avc, &anon_vma->rb_root);

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