On Mon, Nov 10, 2025 at 04:51:27PM +0100, Vlastimil Babka wrote:
> On 11/7/25 17:11, Lorenzo Stoakes wrote:
> > This patch adds the ability to atomically set VMA flags with only the mmap
> > read/VMA read lock held.
> >
> > As this could be hugely problematic for VMA flags in general given that all
> > other accesses are non-atomic and serialised by the mmap/VMA locks, we
> > implement this with a strict allow-list - that is, only designated flags
> > are allowed to do this.
> >
> > We make VM_MAYBE_GUARD one of these flags.
> >
> > Reviewed-by: Pedro Falcato <[email protected]>
> > Reviewed-by: Vlastimil Babka <[email protected]>
> > Signed-off-by: Lorenzo Stoakes <[email protected]>
> > ---
> > include/linux/mm.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 2a5516bff75a..699566c21ff7 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -518,6 +518,9 @@ extern unsigned int kobjsize(const void *objp);
> > /* This mask represents all the VMA flag bits used by mlock */
> > #define VM_LOCKED_MASK (VM_LOCKED | VM_LOCKONFAULT)
> >
> > +/* These flags can be updated atomically via VMA/mmap read lock. */
> > +#define VM_ATOMIC_SET_ALLOWED VM_MAYBE_GUARD
> > +
> > /* Arch-specific flags to clear when updating VM flags on protection
> > change */
> > #ifndef VM_ARCH_CLEAR
> > # define VM_ARCH_CLEAR VM_NONE
> > @@ -860,6 +863,45 @@ static inline void vm_flags_mod(struct vm_area_struct
> > *vma,
> > __vm_flags_mod(vma, set, clear);
> > }
> >
> > +static inline bool __vma_flag_atomic_valid(struct vm_area_struct *vma,
> > + int bit)
> > +{
> > + const vm_flags_t mask = BIT(bit);
> > +
> > + /* Only specific flags are permitted */
> > + if (WARN_ON_ONCE(!(mask & VM_ATOMIC_SET_ALLOWED)))
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +/*
> > + * Set VMA flag atomically. Requires only VMA/mmap read lock. Only specific
> > + * valid flags are allowed to do this.
> > + */
> > +static inline void vma_flag_set_atomic(struct vm_area_struct *vma, int bit)
> > +{
> > + /* mmap read lock/VMA read lock must be held. */
> > + if (!rwsem_is_locked(&vma->vm_mm->mmap_lock))
> > + vma_assert_locked(vma);
> > +
> > + if (__vma_flag_atomic_valid(vma, bit))
> > + set_bit(bit, &vma->__vm_flags);
> > +}
> > +
> > +/*
> > + * Test for VMA flag atomically. Requires no locks. Only specific valid
> > flags
> > + * are allowed to do this.
> > + *
> > + * This is necessarily racey, so callers must ensure that serialisation is
> > + * achieved through some other means, or that races are permissible.
> > + */
> > +static inline bool vma_flag_test_atomic(struct vm_area_struct *vma, int
> > bit)
> > +{
> > + if (__vma_flag_atomic_valid(vma, bit))
> > + return test_bit(bit, &vma->__vm_flags);
> > +}
>
> Hm clang is unhappy here.
>
> ./include/linux/mm.h:932:1: error: non-void function does not return a value
> in all control paths [-Werror,-Wreturn-type]
> 932 | }
> | ^
> 1 error generated.
Yeah fun that gcc doesn't highlight this, god knows why not.
I thought I had fixed this (I remember it coming up in testing) but clearly I
fixed at the wrong commit.
>
> I don't have CONFIG_WERROR enabled though, so not sure why it's not just a
> warning, as the function is unused until patch 5/8 which adds a "return
> false" here. So it's just a potential bisection annoyance with clang.
>
> Andrew could you move that hunk from to this patch? Thanks.
I don't think this is the right solution.
Let's just add a return false. Will send a fix-patch.
>
> > +
> > static inline void vma_set_anonymous(struct vm_area_struct *vma)
> > {
> > vma->vm_ops = NULL;
>
Cheers, Lorenzo
