On Sat, Jul 31, 2021 at 10:53:41AM -0700, Luigi Rizzo wrote: > 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 <lrizzo@xxxxxxxxxx> > 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@xxxxxxxxxx/ 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
