Re: [PATCH] mm/vma: read anon_vma carefully in validate_mm()

[Kiryl Shutsemau <kas@kernel.org>]

On Mon, Nov 02, 2026 at 10:25:20AM +0000, Lorenzo Stoakes (ARM) wrote:
> Commit 38564710f443 ("mm: implement and use vma_has_anon_rmap(), silence
> KCSAN") rather clumsily converted the read of vma->anon_vma into a
> vma_has_anon_rmap() without accounting for the separation between
> assignment of anon_vma and later use of it.
> 
> vma_has_anon_rmap() is intended as an optimistic check, so doesn't need to
> protect against a torn read in vma->anon_vma. The caller is expected to
> take care of this themselves if they then in fact access the field.
> 
> validate_mm() does indeed then reference the field, so resolve the issue
> via READ_ONCE(), and simplify the code by separating this into a new
> function validate_mm_anon_rmap().
> 
> Finally, add a vma_assert_stabilised() to make it clear the required state
> of the VMA.
> 
> Fixes: 38564710f443 ("mm: implement and use vma_has_anon_rmap(), silence KCSAN")
> Reported-by: syzbot+10e72ea5747737bcd2ad@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/6ae78418.6552420f.2a0efc.005c.GAE@google.com/
> Reported-by: Songsen Yang <yangsongsen@insta360.com>
> Closes: https://lore.kernel.org/all/20261102074414.10902-1-yangsongsen@insta360.com/
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
>  mm/vma.c | 36 ++++++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/mm/vma.c b/mm/vma.c
> index d416ba58eb4c..9f02f19a7845 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -669,6 +669,28 @@ static int dup_anon_vma(struct vm_area_struct *dst,
>  }
>  
>  #ifdef CONFIG_DEBUG_VM_MAPLE_TREE
> +#ifdef CONFIG_DEBUG_VM_RB
> +static void validate_mm_anon_rmap(struct vm_area_struct *vma)
> +{
> +	struct anon_vma *anon_vma = READ_ONCE(vma->anon_vma);
> +	struct anon_vma_chain *avc;
> +
> +	vma_assert_stabilised(vma);
> +
> +	if (!anon_vma)
> +		return;
> +
> +	anon_vma_lock_read(anon_vma);
> +	list_for_each_entry(avc, &vma->anon_vma_chain, same_vma)
> +		anon_rmap_tree_verify(avc);
> +	anon_vma_unlock_read(anon_vma);
> +}
> +#else
> +static void validate_mm_anon_rmap(struct vm_area_struct *vma)
> +{
> +}
> +#endif
> +
>  void validate_mm(struct mm_struct *mm)
>  {
>  	int bug = 0;

This ifdeffery seems unnecessary. What about this:

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 9f167950926a..2e43077dcd51 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4363,9 +4363,7 @@ anon_rmap_tree_iter_first(struct anon_vma *anon_vma,
 struct anon_vma_chain *
 anon_rmap_tree_iter_next(struct anon_vma_chain *avc,
 			 pgoff_t pgoff_start, pgoff_t pgoff_last);
-#ifdef CONFIG_DEBUG_VM_RB
 void anon_rmap_tree_verify(struct anon_vma_chain *avc);
-#endif
 
 #define anon_rmap_tree_foreach(avc, anon_vma, pgoff_start, pgoff_last)		 \
 	for (avc = anon_rmap_tree_iter_first(anon_vma, pgoff_start, pgoff_last); \
diff --git a/mm/vma.c b/mm/vma.c
index 1be613044b4a..30f31d3b3e26 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -669,6 +669,26 @@ static int dup_anon_vma(struct vm_area_struct *dst,
 }
 
 #ifdef CONFIG_DEBUG_VM_MAPLE_TREE
+static void validate_mm_anon_rmap(struct vm_area_struct *vma)
+{
+	struct anon_vma *anon_vma;
+	struct anon_vma_chain *avc;
+
+	if (!IS_ENABLED(CONFIG_DEBUG_VM_RB))
+		return;
+
+	vma_assert_stabilised(vma);
+
+	anon_vma = READ_ONCE(vma->anon_vma);
+	if (!anon_vma)
+		return;
+
+	anon_vma_lock_read(anon_vma);
+	list_for_each_entry(avc, &vma->anon_vma_chain, same_vma)
+		anon_rmap_tree_verify(avc);
+	anon_vma_unlock_read(anon_vma);
+}
+
 void validate_mm(struct mm_struct *mm)
 {
 	int bug = 0;
@@ -678,10 +698,6 @@ void validate_mm(struct mm_struct *mm)
 
 	mt_validate(&mm->mm_mt);
 	for_each_vma(vmi, vma) {
-#ifdef CONFIG_DEBUG_VM_RB
-		struct anon_vma *anon_vma = vma->anon_vma;
-		struct anon_vma_chain *avc;
-#endif
 		unsigned long vmi_start, vmi_end;
 		bool warn = 0;
 
@@ -702,14 +718,8 @@ void validate_mm(struct mm_struct *mm)
 			vma_iter_dump_tree(&vmi);
 		}
 
-#ifdef CONFIG_DEBUG_VM_RB
-		if (vma_has_anon_rmap(vma)) {
-			anon_vma_lock_read(anon_vma);
-			list_for_each_entry(avc, &vma->anon_vma_chain, same_vma)
-				anon_rmap_tree_verify(avc);
-			anon_vma_unlock_read(anon_vma);
-		}
-#endif
+		validate_mm_anon_rmap(vma);
+
 		/* Check for a infinite loop */
 		if (++i > mm->map_count + 10) {
 			i = -1;
-- 
  Kiryl Shutsemau / Kirill A. Shutemov