Re: [RFC PATCH v2 09/13] mm: prepare anon_node replacement for ANON_VMA_FRACTAL

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Lockless traversal of the shared `fractal_list` in `anon_node_verify()` leads to data races and potential use-after-free.
- [Low] Using `#if CONFIG_DEBUG_VM_RB` instead of `#ifdef` causes `-Wundef` compilation warnings when the config is disabled.
--

--- Patch [9]: [RFC PATCH v2 09/13] mm: prepare anon_node replacement for ANON_VMA_FRACTAL ---
commit 7ac3c97b907c3a50c0e59be4fc5c3a22d3833da9
Author: tao <tao.wangtao@honor.com>
    
mm: prepare anon_node replacement for ANON_VMA_FRACTAL

This commit introduces macros in vma.c to isolate code differences and
split the large code changes into smaller patches for the anon_node
replacement in ANON_VMA_FRACTAL.

> diff --git a/mm/vma.c b/mm/vma.c
> index d2996369e30d6..02178112daa0b 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c

[ ... ]

> @@ -675,12 +675,19 @@ void validate_mm(struct mm_struct *mm)
>  			vma_iter_dump_tree(&vmi);
>  		}
>  
> -#ifdef CONFIG_DEBUG_VM_RB
> -		if (anon_vma) {
> +#if CONFIG_DEBUG_VM_RB

[Severity: Low]
Does this introduce a -Wundef compiler warning when the config is disabled?

Since CONFIG_DEBUG_VM_RB is a boolean option, it is left undefined rather
than set to 0 when disabled. Using #if instead of #ifdef might cause
build failures when compiling with -Werror.

> +		if (vma->anon_vma) {
> +#ifdef CONFIG_ANON_VMA_FRACTAL
> +			anon_node_verify((void *)vma->anon_vma);

[Severity: Medium]
Can this cause a data race or use-after-free by locklessly iterating over
the fractal_list?

In the legacy path below, anon_vma_lock_read() is explicitly acquired
before traversing the list. However, anon_node_verify() iterates over
the shared anon_node tree using anon_node_next_rbc_child() without
taking anon_node_lock_rmap().

Since the anon_node structure is shared across multiple processes after
fork(), could another process concurrently modify the fractal_list while
validate_mm() is traversing it?

> +#else
> +			struct anon_vma *anon_vma = vma->anon_vma;
> +			struct anon_vma_chain *avc;
> +
>  			anon_vma_lock_read(anon_vma);
>  			list_for_each_entry(avc, &vma->anon_vma_chain, same_vma)
>  				anon_vma_interval_tree_verify(avc);
>  			anon_vma_unlock_read(anon_vma);
> +#endif
>  		}
>  #endif

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