| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210812091742.nbnmsa37adaqkxwd@wittgenstein>
Date: Thu, 12 Aug 2021 11:17:42 +0200
From: Christian Brauner <christian.brauner@...ntu.com>
To: David Hildenbrand <david@...hat.com>
Cc: linux-kernel@...r.kernel.org,
Linus Torvalds <torvalds@...ux-foundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
"H. Peter Anvin" <hpa@...or.com>,
Alexander Viro <viro@...iv.linux.org.uk>,
Alexey Dobriyan <adobriyan@...il.com>,
Steven Rostedt <rostedt@...dmis.org>,
Peter Zijlstra <peterz@...radead.org>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Petr Mladek <pmladek@...e.com>,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Kees Cook <keescook@...omium.org>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Greg Ungerer <gerg@...ux-m68k.org>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Mike Rapoport <rppt@...nel.org>,
Vlastimil Babka <vbabka@...e.cz>,
Vincenzo Frascino <vincenzo.frascino@....com>,
Chinwen Chang <chinwen.chang@...iatek.com>,
Michel Lespinasse <walken@...gle.com>,
Catalin Marinas <catalin.marinas@....com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
Huang Ying <ying.huang@...el.com>,
Jann Horn <jannh@...gle.com>, Feng Tang <feng.tang@...el.com>,
Kevin Brodsky <Kevin.Brodsky@....com>,
Michael Ellerman <mpe@...erman.id.au>,
Shawn Anastasio <shawn@...stas.io>,
Steven Price <steven.price@....com>,
Nicholas Piggin <npiggin@...il.com>,
Jens Axboe <axboe@...nel.dk>,
Gabriel Krisman Bertazi <krisman@...labora.com>,
Peter Xu <peterx@...hat.com>,
Suren Baghdasaryan <surenb@...gle.com>,
Shakeel Butt <shakeelb@...gle.com>,
Marco Elver <elver@...gle.com>,
Daniel Jordan <daniel.m.jordan@...cle.com>,
Nicolas Viennot <Nicolas.Viennot@...sigma.com>,
Thomas Cedeno <thomascedeno@...gle.com>,
Collin Fijalkovich <cfijalkovich@...gle.com>,
Michal Hocko <mhocko@...e.com>,
Miklos Szeredi <miklos@...redi.hu>,
Chengguang Xu <cgxu519@...ernel.net>,
Christian König <ckoenig.leichtzumerken@...il.com>,
linux-unionfs@...r.kernel.org, linux-api@...r.kernel.org,
x86@...nel.org, linux-fsdevel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v1 2/7] kernel/fork: factor out atomcially replacing the
current MM exe_file
On Thu, Aug 12, 2021 at 10:43:43AM +0200, David Hildenbrand wrote:
> Let's factor the main logic out into atomic_set_mm_exe_file(), such that
> all mm->exe_file logic is contained in kernel/fork.c.
>
> While at it, perform some simple cleanups that are possible now that
> we're simplifying the individual functions.
>
> Signed-off-by: David Hildenbrand <david@...hat.com>
> ---
Looks good.
Acked-by: Christian Brauner <christian.brauner@...ntu.com>
> include/linux/mm.h | 2 ++
> kernel/fork.c | 35 +++++++++++++++++++++++++++++++++--
> kernel/sys.c | 33 +--------------------------------
> 3 files changed, 36 insertions(+), 34 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7ca22e6e694a..197505324b74 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2581,6 +2581,8 @@ extern int mm_take_all_locks(struct mm_struct *mm);
> extern void mm_drop_all_locks(struct mm_struct *mm);
>
> extern void set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file);
> +extern int atomic_set_mm_exe_file(struct mm_struct *mm,
> + struct file *new_exe_file);
> extern struct file *get_mm_exe_file(struct mm_struct *mm);
> extern struct file *get_task_exe_file(struct task_struct *task);
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index bc94b2cc5995..6bd2e52bcdfb 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1149,8 +1149,8 @@ void mmput_async(struct mm_struct *mm)
> * Main users are mmput() and sys_execve(). Callers prevent concurrent
> * invocations: in mmput() nobody alive left, in execve task is single
> * threaded. sys_prctl(PR_SET_MM_MAP/EXE_FILE) also needs to set the
> - * mm->exe_file, but does so without using set_mm_exe_file() in order
> - * to avoid the need for any locks.
> + * mm->exe_file, but uses atomic_set_mm_exe_file(), avoiding the need
> + * for any locks.
> */
> void set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file)
> {
> @@ -1170,6 +1170,37 @@ void set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file)
> fput(old_exe_file);
> }
>
> +int atomic_set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file)
> +{
> + struct vm_area_struct *vma;
> + struct file *old_exe_file;
> + int ret = 0;
> +
> + /* Forbid mm->exe_file change if old file still mapped. */
> + old_exe_file = get_mm_exe_file(mm);
> + if (old_exe_file) {
> + mmap_read_lock(mm);
> + for (vma = mm->mmap; vma && !ret; vma = vma->vm_next) {
> + if (!vma->vm_file)
> + continue;
> + if (path_equal(&vma->vm_file->f_path,
> + &old_exe_file->f_path))
> + ret = -EBUSY;
> + }
> + mmap_read_unlock(mm);
> + fput(old_exe_file);
> + if (ret)
> + return ret;
> + }
> +
> + /* set the new file, lockless */
> + get_file(new_exe_file);
> + old_exe_file = xchg(&mm->exe_file, new_exe_file);
> + if (old_exe_file)
> + fput(old_exe_file);
> + return 0;
> +}
> +
> /**
> * get_mm_exe_file - acquire a reference to the mm's executable file
> *
> diff --git a/kernel/sys.c b/kernel/sys.c
> index ef1a78f5d71c..40551b411fda 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -1846,7 +1846,6 @@ SYSCALL_DEFINE1(umask, int, mask)
> static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
> {
> struct fd exe;
> - struct file *old_exe, *exe_file;
> struct inode *inode;
> int err;
>
> @@ -1869,40 +1868,10 @@ static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
> if (err)
> goto exit;
>
> - /*
> - * Forbid mm->exe_file change if old file still mapped.
> - */
> - exe_file = get_mm_exe_file(mm);
> - err = -EBUSY;
> - if (exe_file) {
> - struct vm_area_struct *vma;
> -
> - mmap_read_lock(mm);
> - for (vma = mm->mmap; vma; vma = vma->vm_next) {
> - if (!vma->vm_file)
> - continue;
> - if (path_equal(&vma->vm_file->f_path,
> - &exe_file->f_path))
> - goto exit_err;
> - }
> -
> - mmap_read_unlock(mm);
> - fput(exe_file);
> - }
> -
> - err = 0;
> - /* set the new file, lockless */
> - get_file(exe.file);
> - old_exe = xchg(&mm->exe_file, exe.file);
> - if (old_exe)
> - fput(old_exe);
> + err = atomic_set_mm_exe_file(mm, exe.file);
> exit:
> fdput(exe);
> return err;
> -exit_err:
> - mmap_read_unlock(mm);
> - fput(exe_file);
> - goto exit;
> }
>
> /*
> --
> 2.31.1
>
Powered by blists - more mailing lists