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: <20230707043211.3682710-1-surenb@google.com> <20230707043211.3682710-2-surenb@google.com>
 <20230707194829.u3p5zfjckmh6lktx@revolver>
In-Reply-To: <20230707194829.u3p5zfjckmh6lktx@revolver>
From:   Suren Baghdasaryan <surenb@google.com>
Date:   Fri, 7 Jul 2023 13:15:29 -0700
Message-ID: <CAJuCfpEfhwU3hP+tWi=tpRGm2k6hyV7139oh1CH_E6v331wbeA@mail.gmail.com>
Subject: Re: [PATCH 2/2] mm: lock newly mapped VMA which can be modified after
 it becomes visible
To:     "Liam R. Howlett" <Liam.Howlett@oracle.com>,
        Suren Baghdasaryan <surenb@google.com>,
        akpm@linux-foundation.org, willy@infradead.org, david@redhat.com,
        peterx@redhat.com, vbabka@suse.cz, michel@lespinasse.org,
        jglisse@google.com, mhocko@suse.com, hannes@cmpxchg.org,
        dave@stgolabs.net, ldufour@linux.ibm.com, hughd@google.com,
        punit.agrawal@bytedance.com, lstoakes@gmail.com,
        rientjes@google.com, axelrasmussen@google.com, jannh@google.com,
        shakeelb@google.com, tatashin@google.com, gthelen@google.com,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        stable@vger.kernel.org, kernel-team@android.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Jul 7, 2023 at 7:48=E2=80=AFPM Liam R. Howlett <Liam.Howlett@oracle=
.com> wrote:
>
> * Suren Baghdasaryan <surenb@google.com> [230707 00:32]:
> > mmap_region adds a newly created VMA into VMA tree and might modify it
> > afterwards before dropping the mmap_lock. This poses a problem for page
> > faults handled under per-VMA locks because they don't take the mmap_loc=
k
> > and can stumble on this VMA while it's still being modified. Currently
> > this does not pose a problem since post-addition modifications are done
> > only for file-backed VMAs, which are not handled under per-VMA lock.
> > However, once support for handling file-backed page faults with per-VMA
> > locks is added, this will become a race.
> > Fix this by write-locking the VMA before inserting it into the VMA tree=
.
> > Other places where a new VMA is added into VMA tree do not modify it
> > after the insertion, so do not need the same locking.
> >
> > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > ---
> >  mm/mmap.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index c66e4622a557..84c71431a527 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -2812,6 +2812,8 @@ unsigned long mmap_region(struct file *file, unsi=
gned long addr,
> >       if (vma->vm_file)
> >               i_mmap_lock_write(vma->vm_file->f_mapping);
> >
> > +     /* Lock the VMA since it is modified after insertion into VMA tre=
e */
>
> So it is modified, but that i_mmap_lock_write() directly above this
> comment is potentially moving below the insert and that is why this lock
> is needed.

Correct, we should not rely on i_mmap_lock_write() which can be moved
(and is suggested to be moved in
https://lore.kernel.org/all/20230606124939.93561-1-yu.ma@intel.com/).


>
> > +     vma_start_write(vma);
> >       vma_iter_store(&vmi, vma);
> >       mm->map_count++;
> >       if (vma->vm_file) {
> > --
> > 2.41.0.255.g8b1d071c50-goog
> >
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an=
 email to kernel-team+unsubscribe@android.com.
>