Re: [PATCH] mm/vma: fix NULL anon_vma deref in validate_mm() under mmap read lock

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Mon, Nov 02, 2026 at 03:44:13PM +0800, Songsen Yang wrote:
> Since commit 38564710f443 ("mm: implement and use vma_has_anon_rmap(),
> silence KCSAN"), validate_mm() decides whether to walk a VMA's anon_vma
> chain with vma_has_anon_rmap(), which re-reads vma->anon_vma, but still
> locks the anon_vma pointer it cached at the top of the loop iteration.
>
> vms_complete_munmap_vmas() calls validate_mm() after downgrading the
> mmap lock to a read lock. A page fault on another thread can then
> install vma->anon_vma via __anon_vma_prepare() between the two reads.
> The check sees a non-NULL anon_vma while the cached value is still
> NULL, and anon_vma_lock_read() dereferences it:
>
>   BUG: kernel NULL pointer dereference, address: 0000000000000000
>   RIP: 0010:validate_mm+0x143/0x2a0
>   Call Trace:
>    vms_complete_munmap_vmas+0x201/0x2d0
>    do_vmi_align_munmap+0x285/0x2d0
>    do_vmi_munmap+0xcb/0x190
>    __vm_munmap+0xab/0x190
>    __x64_sys_munmap+0x16/0x20
>
> Read vma->anon_vma once and use that value for both the check and the
> lock, as the code did before. Under the mmap read lock vma->anon_vma
> can only go from NULL to non-NULL, so a non-NULL value read here stays
> valid until validate_mm() is done with it. Keep the data_race()
> annotation that vma_has_anon_rmap() provided, since a concurrent
> install is expected.
>
> Only CONFIG_DEBUG_VM_RB kernels are affected.
>
> Testing: on top of mm-hotfixes-unstable at commit 989bf98b512c
> ("iov_iter: validate extracted segment for bvec alignment"), x86_64
> under QEMU TCG with -smp 2 and CONFIG_DEBUG_VM_RB=y. The race window
> is too narrow to hit with the syzbot reproducer under TCG in the time
> available, so a debug-only mdelay(1) was added before the check to
> widen it: without this patch the reproducer crashes with the splat
> above in its first round; with this patch and the same mdelay(1) it
> completes all rounds without any splat. The mdelay(1) is not part of
> this patch. mm selftests were not run.
>
> This issue was analysed, and the fix, changelog and tests were
> prepared, with the help of an AI coding assistant (Claude); the
> submitter has reviewed the change.
>
> 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/
> Signed-off-by: Songsen Yang <yangsongsen@insta360.com>

This fix looks garbled.

You've no guarantee of what is read and when, the ordering isn't a thing at
all here. If that were an issue, you'd need to use a READ_ONCE().

The fix is also not correct. I'll take this over.

> ---
>  mm/vma.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/mm/vma.c b/mm/vma.c
> index 4672d4205..b674c69f7 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -679,7 +679,7 @@ 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 *anon_vma;
>  		struct anon_vma_chain *avc;
>  #endif
>  		unsigned long vmi_start, vmi_end;
> @@ -703,7 +703,13 @@ void validate_mm(struct mm_struct *mm)
>  		}
>
>  #ifdef CONFIG_DEBUG_VM_RB
> -		if (vma_has_anon_rmap(vma)) {
> +		/*
> +		 * Callers may hold only the mmap read lock, in which case a
> +		 * concurrent fault can install vma->anon_vma at any time. Read
> +		 * it once, so that the check and the lock use the same value.
> +		 */
> +		anon_vma = data_race(vma->anon_vma);
> +		if (anon_vma) {
>  			anon_vma_lock_read(anon_vma);
>  			list_for_each_entry(avc, &vma->anon_vma_chain, same_vma)
>  				anon_rmap_tree_verify(avc);
> --
> 2.50.1

--
Cheers, Lorenzo