Re: [PATCH v2] futex: Use-after-free between futex_key_to_node_opt and vma_replace_policy

[Peter Zijlstra]

On Wed, Mar 25, 2026 at 08:19:24AM -0700, Eric Dumazet wrote:
> On Wed, Mar 25, 2026 at 8:14 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> >
> > On Tue, Mar 24, 2026 at 09:27:41PM +0100, David Hildenbrand (Arm) wrote:
> > > So IIUC, futex_key_to_node_opt() looks up a VMA under RCU, without
> > > holding the mmap lock. Concurrent mmap-write lock is detected by using
> > > the mmap_lock_speculate_try_begin()/mmap_lock_speculate_retry() seqcount.
> > >
> > > After looking up the VMA, we access the VMA policy.
> > >
> > > vma_policy() does a straight vma->vm_policy.
> > >
> > > What prevents the compiler here to do some load tearing while it is
> > > getting modified by mbind()? Or what stops the writer side to to some
> > > store tearing?
> > >
> > > Shouldn't we be using at least READ_ONCE/WRITE_ONCE() etc?
> >
> > Bah, at that point we might as well RCU the thing like so, I suppose.
> >
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -1026,7 +1026,7 @@ static int vma_replace_policy(struct vm_
> >         }
> >
> >         old = vma->vm_policy;
> > -       vma->vm_policy = new; /* protected by mmap_lock */
> > +       rcu_assign_pointer(vma->vm_policy, new); /* protected by mmap_lock */
> >         mpol_put(old);
> >
> >         return 0;
> > diff --git a/kernel/futex/core.c b/kernel/futex/core.c
> > index 4bacf5565368..6336a80e3dca 100644
> > --- a/kernel/futex/core.c
> > +++ b/kernel/futex/core.c
> > @@ -342,7 +342,7 @@ static int __futex_key_to_node(struct mm_struct *mm, unsigned long addr)
> >         if (!vma)
> >                 return FUTEX_NO_NODE;
> >
> > -       mpol = vma_policy(vma);
> > +       mpol = rcu_dereference_raw(vma->vm_policy);
> >         if (!mpol)
> >                 return FUTEX_NO_NODE;
> 
> Yes, but sparse will bite :)

Oh gawd, yes, and then people will go 'fix' it and it'll turn into an
unholy mess.

> READ_ONCE()/WRITE_ONCE() on these two locations seems acceptable.

Fair enough. Like so then..

--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -342,7 +342,7 @@ static int __futex_key_to_node(struct mm
 	if (!vma)
 		return FUTEX_NO_NODE;
 
-	mpol = vma_policy(vma);
+	mpol = READ_ONCE(vma->vm_policy);
 	if (!mpol)
 		return FUTEX_NO_NODE;
 
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1026,7 +1026,7 @@ static int vma_replace_policy(struct vm_
 	}
 
 	old = vma->vm_policy;
-	vma->vm_policy = new; /* protected by mmap_lock */
+	WRITE_ONCE(vma->vm_policy, new); /* protected by mmap_lock */
 	mpol_put(old);
 
 	return 0;