Re: [PATCH] mm: fix vma_start_write_killable() signal handling

[Matthew Wilcox <willy@infradead.org>]

On Wed, Nov 26, 2025 at 03:36:46PM +0100, Vlastimil Babka wrote:
> On 11/26/25 5:28 AM, Suren Baghdasaryan wrote:
> >> Suren, Liam, Vlastimil, Lorenzo ... none of you spotted this bug.
> > 
> > Doh! This is embarassing...
> 
> Hand-rolled synchronization primitives are wonderful, aren't they?

That's why I liked the original approach of just using rwsems.  I
mst admit to having not paid attention to this recently so I don't
know what motivated the change.

> > Wait, why do we consider this as a successful acquisition? The
> > vm_refcnt is 0, so this is similar situation to an earlier:
> > 
> > if (!refcount_add_not_zero(VMA_LOCK_OFFSET, &vma->vm_refcnt))
> >         return 0;
> 
> But this means "vma is not attached" not "we failed to lock it".
> 
> > IOW, the vma is not referenced, so we failed to lock it. I think the
> > fix should be:
> > 
> >         if (err) {
> > +               if (refcount_sub_and_test(VMA_LOCK_OFFSET, &vma->vm_refcnt)) {
> > +                       /* Oh cobblers.  While we got a fatal signal, we
> > +                        * raced with the last user.  VMA is not referenced,
> > +                        * fail to lock it.
> > +                        */
> > +                       err = 0;
> 
> Returning 0 in this situation therefore wouldn't be correct.
> 
> AFAIU since we started with attached vma above, it's not possible that
> the refcount_sub_and_test here will drop the refcnt to zero. We could
> just WARN_ON_ONCE() on the result (in a way to make also the
> __must_check happy) and then can return err below.

But how do we know that we started with an attached VMA?  Maybe the VMA
was in the process of being detached and still has readers?