Re: [PATCH] mm: fix the race on huge alloc failed

[Matthew Wilcox <willy@infradead.org>]

First, I hope you're a human being and not just doing what an LLM tells
you, because I'm putting effort into this.  Second, for the same reason,
I hope you stick around and make further contributions.

On Sat, Aug 29, 2026 at 03:02:34PM -0300, Guilherme Giacomo Simoes wrote:
> Matthew Wilcox <willy@infradead.org> wrotes:
> >> Fixes: 164b06f238b9 ("mm: call wp_page_copy() under the VMA lock")
> >
> > what makes you think this is the right commit for fixes?
> Maybe I would should analyzed this better. I only seed the commit that introduce
> this function (and consequently this reader)

That was what I thought, but it's not enough to determine if that's the
start of the problem.  Look, that commit does:

-       if (unlikely(anon_vma_prepare(vma)))
-               goto oom;
+       ret = vmf_anon_prepare(vmf);
+       if (unlikely(ret))
+               goto out;

... and anon_vma_prepare() does:

        if (likely(vma->anon_vma))
                return 0;

so either this race was already present in 164b06f238b9 (and you need to
go back further) or it was actually introduced later (maybe the write
side was introduced later?)

> >> The race occurs because the reader (__vmf_anon_prepare()) checks
> >> `vma->anon->vma` without holding the mmap_lock and withou the
> >> READ_ONCE() macro. Since the writer (__anon_vma_prepare()) is holding the
> >> mmap_lock and updating the pointer, it creates a data race as the two
> >> access are not properly synchronized.
> >
> > also this explanation is bogus.  i don't have time to fix it right now.
> Hmm... I would like to say that the reader (__vmf_anon_prepare) access the same
> data that the writer (__anon_vma_prepare()), lead to a race condition problem.
> 
> When the huge page alloc failed, the asm_exc_page_fault interrupt is fired but
> on the same time the procces that was trying to alloc the huge page, try handle
> to this failed too..
> 
> How READ_ONCE() and WRITE_ONCE() is atomic, the race problem can be resolved.

The important thing to know is that the mmap_lock is a read-write lock.
That means that two readers can be present at the same time.  So this race
can happen when both threads hold the mmap_lock.  I don't know whether
they do in the syzbot reproducer; probably not, but it doesn't matter.

The other important thing is that _we don't care_ what the value of
vma->anon_vma is.  We only care whether it's NULL or not (this is a
sufficiently common case that I wonder whether KCSAN shouldn't special-case
it and decline to monitor it ...)  VMAs are created with a NULL anon_vma,
and then if needed, anon_vma is set.  Once set, it is never changed (uhh
... at least I don't think it is.  Lorenzo, could you check me on this?
I think all the places where we set vma->anon_vma to NULL are in
situations where the VMA is not yet exposed to the page fault handler,
like in the child side of fork()).

So it's inappropriate to use READ_ONCE() / WRITE_ONCE() to "solve"
this problem, because we don't need those semantics.  It's sufficient
to wrap the read side in data_race() to indicate to KCSAN that we know
what we're doing.

Also, as Lance said, I don't see how this is related to huge_page_alloc
failing.  All I see is two threads calling  __vmf_anon_prepare() at the
same time, which I presume is an attempt to COW a hugetlb page.

I don't think it's enough to just add a data_race() to this one read of
vma->anon_vma.  I think it's quite prevalent.  There's probably other
syzbot reports that mention it.