From: Jann Horn <jannh@google.com>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Linux-MM <linux-mm@kvack.org>,
kernel list <linux-kernel@vger.kernel.org>,
"Eric W . Biederman" <ebiederm@xmission.com>,
Michel Lespinasse <walken@google.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Sakari Ailus <sakari.ailus@linux.intel.com>
Subject: Re: [PATCH 3/4] mmap locking API: Don't check locking if the mm isn't live yet
Date: Wed, 30 Sep 2020 14:50:28 +0200 [thread overview]
Message-ID: <CAG48ez2z1R8MLS0_pjhBSGnJC8RwaHMpiGdv8GhJUCrwtsLOPg@mail.gmail.com> (raw)
In-Reply-To: <20200930123000.GC9916@ziepe.ca>
On Wed, Sep 30, 2020 at 2:30 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Tue, Sep 29, 2020 at 06:20:00PM -0700, Jann Horn wrote:
> > In preparation for adding a mmap_assert_locked() check in
> > __get_user_pages(), teach the mmap_assert_*locked() helpers that it's fine
> > to operate on an mm without locking in the middle of execve() as long as
> > it hasn't been installed on a process yet.
>
> I'm happy to see lockdep being added here, but can you elaborate on
> why add this mmap_locked_required instead of obtaining the lock in the
> execv path?
My thinking was: At that point, we're logically still in the
single-owner initialization phase of the mm_struct. Almost any object
has initialization and teardown steps that occur in a context where
the object only has a single owner, and therefore no locking is
required. It seems to me that adding locking in places like
get_arg_page() would be confusing because it would suggest the
existence of concurrency where there is no actual concurrency, and it
might be annoying in terms of lockdep if someone tries to use
something like get_arg_page() while holding the mmap_sem of the
calling process. It would also mean that we'd be doing extra locking
in normal kernel builds that isn't actually logically required.
Hmm, on the other hand, dup_mmap() already locks the child mm (with
mmap_write_lock_nested()), so I guess it wouldn't be too bad to also
do it in get_arg_page() and tomoyo_dump_page(), with comments that
note that we're doing this for lockdep consistency... I guess I can go
change this in v2.
next prev parent reply other threads:[~2020-09-30 12:50 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200930011944.19869-1-jannh@google.com>
2020-09-30 1:20 ` [PATCH 2/4] binfmt_elf: Take the mmap lock around find_extend_vma() Jann Horn
2020-09-30 23:22 ` Michel Lespinasse
2020-09-30 1:20 ` [PATCH 3/4] mmap locking API: Don't check locking if the mm isn't live yet Jann Horn
2020-09-30 12:30 ` Jason Gunthorpe
2020-09-30 12:50 ` Jann Horn [this message]
2020-09-30 20:14 ` Jann Horn
2020-09-30 23:26 ` Jason Gunthorpe
2020-09-30 23:51 ` Jann Horn
2020-10-01 19:15 ` Jason Gunthorpe
2020-10-01 20:16 ` Jann Horn
2020-10-01 23:41 ` Jason Gunthorpe
2020-10-01 23:55 ` Jann Horn
2020-09-30 23:42 ` Michel Lespinasse
2020-09-30 1:20 ` [PATCH 4/4] mm/gup: Assert that the mmap lock is held in __get_user_pages() Jann Horn
2020-09-30 12:32 ` Jason Gunthorpe
2020-09-30 23:24 ` Michel Lespinasse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAG48ez2z1R8MLS0_pjhBSGnJC8RwaHMpiGdv8GhJUCrwtsLOPg@mail.gmail.com \
--to=jannh@google.com \
--cc=akpm@linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=jgg@ziepe.ca \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mchehab@kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=walken@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox