Re: [PATCH 20/30] mm/vma: introduce vma_assert_can_modify()

[Lorenzo Stoakes <ljs@kernel.org>]

On Thu, Jul 02, 2026 at 12:16:32PM +0100, Pedro Falcato wrote:
> On Mon, Jun 29, 2026 at 01:23:31PM +0100, Lorenzo Stoakes wrote:
> > vma_assert_write_locked() and vma_assert_attached() are useful for their
> > own purposes, however VMA code absolutely does allow the modification of
> > non-write locked VMAs if they are at that point detached (i.e. unreachable
> > from anywhere).
> >
> > It's therefore useful to be able to assert that a VMA is either
> > detached (modification doesn't matter) or write locked (you're explicitly
> > locked for modification).
>
> Hmm, I was wondering why detached does not imply write_locked, and then

For one obviously when detaching they are also write locked (see
vma_mark_detached()) but yeah the point of this is when you have a VMA allocated
which doesn't hold the appropriate lock.

> realized that new VMAs aren't write-locked. Could we do it by default?
> Like a simple:
>
> 	vma->vm_lock_seq = __vma_raw_mm_seqnum(vma);
>
> might do the trick. I don't see why it wouldn't work? Is there some other
> case I am not considering?

Firstly, I'd rather not rework the VMA lock logic as part of this series. It's
subtle and such changes are tricky.

I'm trying to achieve the minimum changes while adding extra validation as I can
in preparation for the scalable CoW work.

And I'm not sure wanting to alter the fundamentals of VMA locks is a good reason
not to ack a patch :)

But since you bring it up...

There's subtleties here too.

- At what point do you do this assignment? If it's the VMA allocation logic,
  it's not obvious then that you even have the mmap write lock necessarily. Now
  you're making assumptions that might be broken, or broken in future.

  Now everybody who allocates a VMA has to 'just know' that they need to assign
  an mm and mark it write-locked (but in a special new VMA way) before setting
  the pgoff.

- Detached means it is not currently in any tree, nor belongs necessarily to any
  mm. So the concept of it being write locked is meaningless.

- It's broken to perform actions on a new VMA that is not yet linked into any
  tree that would require the VMA write lock. Currently the code _explicitly_
  asserts that a detached VMA is not attempted to be write locked.

- So we have a good way of catching people doing stupid or broken stuff to VMAs
  that are not in the correct state (we currently _don't_ do that for detached
  VMAs that _were_ in the tree, probably we should change that...!)

- You'd have to create a new function to do this since we explicitly disallow
  doing this right now, and that's more complexity and then you're then
  creating a whole new meaning as to what VMA write lock acquisition is,
  which is even more added complexity.

- create_init_stack_vma() would break, so would hugetlb (lol) and static gate
  VMAs explicitly do not belong to an mm, so there's simply no concept of them
  being VMA write locked anyway.

- I'm not sure violating the invariant of seqnum = 0 = no write locked VMAs is
  safe.

- Things get quite horrendous on fork (prior to us taking tmp's VMA write lock)
  - you'd have to assign this field after duplicating the VMA midway through
  attaching it to the new mm, where that mm has a new seqnum. But the new mm has
  a seqnum of 0... so now you're having to duplicate it but alter what
  vma_lock_init() does to reset the seqnum to 0 but then what if the
  'duplication' logic asserts VMA write locked?

It's very chicken and egg - on VMA duplication you want to be able to write the
very fields that you need to do anything with the VMA prior to it being linked
in anywhere.

And we probably don't want to bake in the assumption that to change fundamental
fields requires that you always hold the write lock as a consequence.

>
> >
> > Therefore introduce vma_assert_can_modify() for this purpose.
> >
> > While we're here, make vma_is_attached() available generally - if
> > !CONFIG_PER_VMA_LOCKS, then there's no sense in which a VMA is
> > detached (vma_mark_detached() is a noop), so have this default to true in
> > this case.
> >
> > Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> > ---
> >  include/linux/mmap_lock.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> > index 04b8f61ece5d..d513286d8160 100644
> > --- a/include/linux/mmap_lock.h
> > +++ b/include/linux/mmap_lock.h
> > @@ -506,6 +506,8 @@ static inline __must_check
> >  int vma_start_write_killable(struct vm_area_struct *vma) { return 0; }
> >  static inline void vma_assert_write_locked(struct vm_area_struct *vma)
> >  		{ mmap_assert_write_locked(vma->vm_mm); }
> > +static inline bool vma_is_attached(struct vm_area_struct *vma)
> > +		{ return true; }
> >  static inline void vma_assert_attached(struct vm_area_struct *vma) {}
> >  static inline void vma_assert_detached(struct vm_area_struct *vma) {}
> >  static inline void vma_mark_attached(struct vm_area_struct *vma) {}
> > @@ -530,6 +532,12 @@ static inline void vma_assert_stabilised(struct vm_area_struct *vma)
> >
> >  #endif /* CONFIG_PER_VMA_LOCK */
> >
> > +static inline void vma_assert_can_modify(struct vm_area_struct *vma)
> > +{
> > +	if (vma_is_attached(vma))
> > +		vma_assert_write_locked(vma);
> > +}
> > +
> >  static inline void mmap_write_lock(struct mm_struct *mm)
> >  {
> >  	__mmap_lock_trace_start_locking(mm, true);
> > --
> > 2.54.0
> >
>
> --
> Pedro

Thanks, Lorenzo