On Thu, Jul 27, 2023 at 5:21 PM Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Matthew,
>
> On Fri, 28 Jul 2023 00:49:50 +0100 Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> >
> > On Fri, Jul 28, 2023 at 09:18:49AM +1000, Stephen Rothwell wrote:
> > > diff --cc mm/memory.c
> > > index ca632b58f792,271982fab2b8..000000000000
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@@ -5392,32 -5597,18 +5597,21 @@@ retry
> > > if (!vma)
> > > goto inval;
> > >
> > > - /* Only anonymous and tcp vmas are supported for now */
> > > - if (!vma_is_anonymous(vma) && !vma_is_tcp(vma))
> > > - /* find_mergeable_anon_vma uses adjacent vmas which are not locked */
> > > - if (vma_is_anonymous(vma) && !vma->anon_vma)
> > > -- goto inval;
> > > --
> > > if (!vma_start_read(vma))
> > > goto inval;
> > >
> > > + /*
> > > + * find_mergeable_anon_vma uses adjacent vmas which are not locked.
> > > + * This check must happen after vma_start_read(); otherwise, a
> > > + * concurrent mremap() with MREMAP_DONTUNMAP could dissociate the VMA
> > > + * from its anon_vma.
> > > + */
> > > - if (unlikely(!vma->anon_vma && !vma_is_tcp(vma)))
> > > - goto inval_end_read;
> > > -
> > > - /*
> > > - * Due to the possibility of userfault handler dropping mmap_lock, avoid
> > > - * it for now and fall back to page fault handling under mmap_lock.
> > > - */
> > > - if (userfaultfd_armed(vma))
> > > ++ if (unlikely(vma_is_anonymous(vma) && !vma_is_tcp(vma)))
> > > + goto inval_end_read;
> > > +
> >
> > No, this isn't right. It should be:
> >
> > if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma))
> > goto inval_end_read;
>
> Yeah, see my second attempt.
>
> > I'm not sure about the userfaultfd_armed() clause. My patch wasn't
> > intended to affect that.
>
> That was removed by commit
>
> 69f6bbd1317f ("mm: handle userfaults under VMA lock")
>
> in the mm branch.
Hmm. 657b5146955e ("mm: lock_vma_under_rcu() must check vma->anon_vma
under vma lock") should be adding a "inval_end_read" label. At least I
see it in https://lore.kernel.org/all/20230726214103.3261108-3-jannh@xxxxxxxxxx/
and will check Linus' tree in a min. I don't see that label in your
patch...
I also misspoke in my previous message. The patches in mm tree remove
some code from that function, so it's easier to apply them first and
then the one from Linus' tree.
>
> --
> Cheers,
> Stephen Rothwell
