Re: [PATCH] Add mmap_assert_locked() annotations to find_vma*()
From: Jason Gunthorpe <jgg@ziepe.ca>
Date: 2021-08-03 16:08:18
Also in:
lkml
On Sat, Jul 31, 2021 at 10:53:41AM -0700, Luigi Rizzo wrote:
quoted hunk ↗ jump to hunk
find_vma() and variants need protection when used. This patch adds mmap_assert_lock() calls in the functions. To make sure the invariant is satisfied, we also need to add a mmap_read_loc() around the get_user_pages_remote() call in get_arg_page(). The lock is not strictly necessary because the mm has been newly created, but the extra cost is limited because the same mutex was also acquired shortly before in __bprm_mm_init(), so it is hot and uncontended. Signed-off-by: Luigi Rizzo <redacted> fs/exec.c | 2 ++ mm/mmap.c | 2 ++ 2 files changed, 4 insertions(+)diff --git a/fs/exec.c b/fs/exec.c index 38f63451b928..ac7603e985b4 100644 +++ b/fs/exec.c@@ -217,8 +217,10 @@ static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, * We are doing an exec(). 'current' is the process * doing the exec and bprm->mm is the new process's mm. */ + mmap_read_lock(bprm->mm); ret = get_user_pages_remote(bprm->mm, pos, 1, gup_flags, &page, NULL, NULL); + mmap_read_unlock(bprm->mm); if (ret <= 0) return NULL;
Wasn't Jann Horn working on something like this too? https://lore.kernel.org/linux-mm/20201016225713.1971256-1-jannh@google.com/ (local) IIRC it was very tricky here, are you sure it is OK to obtain this lock here? I would much rather see Jann's complete solution be merged then hacking at the exec problem on the side.. Jason