* [PATCH] mm/vma: read anon_vma carefully in validate_mm()
@ 2026-11-02 10:25 Lorenzo Stoakes (ARM)
2026-11-02 10:36 ` Pedro Falcato
2026-11-02 10:48 ` Kiryl Shutsemau
0 siblings, 2 replies; 5+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-11-02 10:25 UTC (permalink / raw)
To: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn,
Pedro Falcato, Kiryl Shutsemau (Meta), David Hildenbrand (Arm)
Cc: Lance Yang, linux-mm, linux-kernel, ljs,
syzbot+10e72ea5747737bcd2ad, Songsen Yang
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>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mm/vma: read anon_vma carefully in validate_mm() 2026-11-02 10:25 [PATCH] mm/vma: read anon_vma carefully in validate_mm() Lorenzo Stoakes (ARM) @ 2026-11-02 10:36 ` Pedro Falcato 2026-11-02 10:48 ` Kiryl Shutsemau 1 sibling, 0 replies; 5+ messages in thread From: Pedro Falcato @ 2026-11-02 10:36 UTC (permalink / raw) To: Lorenzo Stoakes (ARM) Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn, Kiryl Shutsemau (Meta), David Hildenbrand (Arm), Lance Yang, linux-mm, linux-kernel, syzbot+10e72ea5747737bcd2ad, Songsen Yang 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vma: read anon_vma carefully in validate_mm() 2026-11-02 10:25 [PATCH] mm/vma: read anon_vma carefully in validate_mm() Lorenzo Stoakes (ARM) 2026-11-02 10:36 ` Pedro Falcato @ 2026-11-02 10:48 ` Kiryl Shutsemau 2026-11-02 11:02 ` Lorenzo Stoakes (ARM) 1 sibling, 1 reply; 5+ messages in thread From: Kiryl Shutsemau @ 2026-11-02 10:48 UTC (permalink / raw) To: Lorenzo Stoakes (ARM) Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato, David Hildenbrand (Arm), Lance Yang, linux-mm, linux-kernel, syzbot+10e72ea5747737bcd2ad, Songsen Yang 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vma: read anon_vma carefully in validate_mm() 2026-11-02 10:48 ` Kiryl Shutsemau @ 2026-11-02 11:02 ` Lorenzo Stoakes (ARM) 2026-11-02 21:24 ` Andrew Morton 0 siblings, 1 reply; 5+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-11-02 11:02 UTC (permalink / raw) To: Kiryl Shutsemau Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato, David Hildenbrand (Arm), Lance Yang, linux-mm, linux-kernel, syzbot+10e72ea5747737bcd2ad, Songsen Yang 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vma: read anon_vma carefully in validate_mm() 2026-11-02 11:02 ` Lorenzo Stoakes (ARM) @ 2026-11-02 21:24 ` Andrew Morton 0 siblings, 0 replies; 5+ messages in thread From: Andrew Morton @ 2026-11-02 21:24 UTC (permalink / raw) To: Lorenzo Stoakes (ARM) Cc: Kiryl Shutsemau, Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato, David Hildenbrand (Arm), Lance Yang, linux-mm, linux-kernel, syzbot+10e72ea5747737bcd2ad, Songsen Yang On Mon, 2 Nov 2026 11:02:56 +0000 "Lorenzo Stoakes (ARM)" <ljs@kernel.org> wrote: > > This ifdeffery seems unnecessary. What about this: > > Yup, even better. > > Andrew - could you update it in-place or do you need a v2? I queued this update: --- a/include/linux/mm.h~mm-vma-read-anon_vma-carefully-in-validate_mm-fix +++ a/include/linux/mm.h @@ -4370,9 +4370,7 @@ anon_rmap_tree_iter_first(struct anon_vm 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); \ --- a/mm/vma.c~mm-vma-read-anon_vma-carefully-in-validate_mm-fix +++ a/mm/vma.c @@ -669,14 +669,17 @@ static int dup_anon_vma(struct vm_area_s } #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 *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; @@ -685,11 +688,6 @@ static void validate_mm_anon_rmap(struct 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) { _ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-02 21:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-11-02 10:25 [PATCH] mm/vma: read anon_vma carefully in validate_mm() Lorenzo Stoakes (ARM) 2026-11-02 10:36 ` Pedro Falcato 2026-11-02 10:48 ` Kiryl Shutsemau 2026-11-02 11:02 ` Lorenzo Stoakes (ARM) 2026-11-02 21:24 ` Andrew Morton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®