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

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

[Lorenzo Stoakes (ARM)]

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;
@@ -678,10 +700,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 +720,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;

---
base-commit: b61fa14ce6bcbba2cb9a96feea31b760226d6b43
change-id: 20261102-fix-validate_mm-b7930d41388b

Cheers,
-- 
Lorenzo Stoakes (ARM) <ljs@kernel.org>

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

[Pedro Falcato]

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>

Reviewed-by: Pedro Falcato <pfalcato@suse.de> 

-- 
Pedro

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

[Kiryl Shutsemau]

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

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

[Lorenzo Stoakes (ARM)]

On Mon, Nov 02, 2026 at 10:48:45AM +0000, Kiryl Shutsemau wrote:
> 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:

Yup, even better.

Andrew - could you update it in-place or do you need a v2?

>
> 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

--
Cheers, Lorenzo