Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230728091849.7f32259d@canb.auug.org.au> <ZMMCnvc3h7XT/NM2@casper.infradead.org>
 <20230728102022.7593856b@canb.auug.org.au>
In-Reply-To: <20230728102022.7593856b@canb.auug.org.au>
From:   Suren Baghdasaryan <surenb@google.com>
Date:   Thu, 27 Jul 2023 17:23:45 -0700
Message-ID: <CAJuCfpHzz3f8Ru7C03b-TVi0g1gcMOEkd2v=ayNunEEs6cjZxg@mail.gmail.com>
Subject: Re: linux-next: manual merge of the mm tree with Linus' tree
To:     Stephen Rothwell <sfr@canb.auug.org.au>
Cc:     Matthew Wilcox <willy@infradead.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Jann Horn <jannh@google.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux Next Mailing List <linux-next@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Jul 27, 2023 at 5:21=E2=80=AFPM Stephen Rothwell <sfr@canb.auug.org=
.au> wrote:
>
> Hi Matthew,
>
> On Fri, 28 Jul 2023 00:49:50 +0100 Matthew Wilcox <willy@infradead.org> w=
rote:
> >
> > 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 locke=
d */
> > >  -  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 locke=
d.
> > >  +   * 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_loc=
k.
> > > -    */
> > > -   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@google=
.com/
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