execve: use 'struct filename *' for executable name passing

This changes 'do_execve()' to get the executable name as a 'struct
filename', and to free it when it is done.  This is what the normal
users want, and it simplifies and streamlines their error handling.

The controlled lifetime of the executable name also fixes a
use-after-free problem with the trace_sched_process_exec tracepoint: the
lifetime of the passed-in string for kernel users was not at all
obvious, and the user-mode helper code used UMH_WAIT_EXEC to serialize
the pathname allocation lifetime with the execve() having finished,
which in turn meant that the trace point that happened after
mm_release() of the old process VM ended up using already free'd memory.

To solve the kernel string lifetime issue, this simply introduces
"getname_kernel()" that works like the normal user-space getname()
function, except with the source coming from kernel memory.

As Oleg points out, this also means that we could drop the tcomm[] array
from 'struct linux_binprm', since the pathname lifetime now covers
setup_new_exec().  That would be a separate cleanup.

Reported-by: Igor Zhbanov <i.zhbanov@samsung.com>
Tested-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/arch/parisc/hpux/fs.c b/arch/parisc/hpux/fs.c
index 88d0962de65a8ca97c877b90dfb0b9ac59b0bfac..2bedafea3d94c13c7215a4be242f12e7a2f185f1 100644 (file)
--- a/arch/parisc/hpux/fs.c
+++ b/arch/parisc/hpux/fs.c
@@ -33,22 +33,9 @@
 
 int hpux_execve(struct pt_regs *regs)
 {
-       int error;
-       struct filename *filename;
-
-       filename = getname((const char __user *) regs->gr[26]);
-       error = PTR_ERR(filename);
-       if (IS_ERR(filename))
-               goto out;
-
-       error = do_execve(filename->name,
+       return  do_execve(getname((const char __user *) regs->gr[26]),
                          (const char __user *const __user *) regs->gr[25],
                          (const char __user *const __user *) regs->gr[24]);
-
-       putname(filename);
-
-out:
-       return error;
 }
 
 struct hpux_dirent {

diff --git a/fs/exec.c b/fs/exec.c
index e1529b4c79b1c29b300ab6519a94d7b748069e33..3d78fccdd723e21119b6c93c70e2564a11d0e6a3 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -748,11 +748,10 @@ EXPORT_SYMBOL(setup_arg_pages);
 
 #endif /* CONFIG_MMU */
 
-struct file *open_exec(const char *name)
+static struct file *do_open_exec(struct filename *name)
 {
        struct file *file;
        int err;
-       struct filename tmp = { .name = name };
        static const struct open_flags open_exec_flags = {
                .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
                .acc_mode = MAY_EXEC | MAY_OPEN,
@@ -760,7 +759,7 @@ struct file *open_exec(const char *name)
                .lookup_flags = LOOKUP_FOLLOW,
        };
 
-       file = do_filp_open(AT_FDCWD, &tmp, &open_exec_flags);
+       file = do_filp_open(AT_FDCWD, name, &open_exec_flags);
        if (IS_ERR(file))
                goto out;
 
@@ -784,6 +783,12 @@ exit:
        fput(file);
        return ERR_PTR(err);
 }
+
+struct file *open_exec(const char *name)
+{
+       struct filename tmp = { .name = name };
+       return do_open_exec(&tmp);
+}
 EXPORT_SYMBOL(open_exec);
 
 int kernel_read(struct file *file, loff_t offset,
@@ -1162,7 +1167,7 @@ int prepare_bprm_creds(struct linux_binprm *bprm)
        return -ENOMEM;
 }
 
-void free_bprm(struct linux_binprm *bprm)
+static void free_bprm(struct linux_binprm *bprm)
 {
        free_arg_pages(bprm);
        if (bprm->cred) {
@@ -1432,7 +1437,7 @@ static int exec_binprm(struct linux_binprm *bprm)
 /*
  * sys_execve() executes a new program.
  */
-static int do_execve_common(const char *filename,
+static int do_execve_common(struct filename *filename,
                                struct user_arg_ptr argv,
                                struct user_arg_ptr envp)
 {
@@ -1441,6 +1446,9 @@ static int do_execve_common(const char *filename,
        struct files_struct *displaced;
        int retval;
 
+       if (IS_ERR(filename))
+               return PTR_ERR(filename);
+
        /*
         * We move the actual failure in case of RLIMIT_NPROC excess from
         * set*uid() to execve() because too many poorly written programs
@@ -1473,7 +1481,7 @@ static int do_execve_common(const char *filename,
        check_unsafe_exec(bprm);
        current->in_execve = 1;
 
-       file = open_exec(filename);
+       file = do_open_exec(filename);
        retval = PTR_ERR(file);
        if (IS_ERR(file))
                goto out_unmark;
@@ -1481,8 +1489,7 @@ static int do_execve_common(const char *filename,
        sched_exec();
 
        bprm->file = file;
-       bprm->filename = filename;
-       bprm->interp = filename;
+       bprm->filename = bprm->interp = filename->name;
 
        retval = bprm_mm_init(bprm);
        if (retval)
@@ -1523,6 +1530,7 @@ static int do_execve_common(const char *filename,
        acct_update_integrals(current);
        task_numa_free(current);
        free_bprm(bprm);
+       putname(filename);
        if (displaced)
                put_files_struct(displaced);
        return retval;
@@ -1544,10 +1552,11 @@ out_files:
        if (displaced)
                reset_files_struct(displaced);
 out_ret:
+       putname(filename);
        return retval;
 }
 
-int do_execve(const char *filename,
+int do_execve(struct filename *filename,
        const char __user *const __user *__argv,
        const char __user *const __user *__envp)
 {
@@ -1557,7 +1566,7 @@ int do_execve(const char *filename,
 }
 
 #ifdef CONFIG_COMPAT
-static int compat_do_execve(const char *filename,
+static int compat_do_execve(struct filename *filename,
        const compat_uptr_t __user *__argv,
        const compat_uptr_t __user *__envp)
 {
@@ -1607,25 +1616,13 @@ SYSCALL_DEFINE3(execve,
                const char __user *const __user *, argv,
                const char __user *const __user *, envp)
 {
-       struct filename *path = getname(filename);
-       int error = PTR_ERR(path);
-       if (!IS_ERR(path)) {
-               error = do_execve(path->name, argv, envp);
-               putname(path);
-       }
-       return error;
+       return do_execve(getname(filename), argv, envp);
 }
 #ifdef CONFIG_COMPAT
 asmlinkage long compat_sys_execve(const char __user * filename,
        const compat_uptr_t __user * argv,
        const compat_uptr_t __user * envp)
 {
-       struct filename *path = getname(filename);
-       int error = PTR_ERR(path);
-       if (!IS_ERR(path)) {
-               error = compat_do_execve(path->name, argv, envp);
-               putname(path);
-       }
-       return error;
+       return compat_do_execve(getname(filename), argv, envp);
 }
 #endif

diff --git a/fs/namei.c b/fs/namei.c
index d580df2e6804d0863d555234388a34397387c3e6..385f7817bfccbd12fbc352c39827586d4cf953d7 100644 (file)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -196,6 +196,7 @@ recopy:
                goto error;
 
        result->uptr = filename;
+       result->aname = NULL;
        audit_getname(result);
        return result;
 
@@ -210,6 +211,35 @@ getname(const char __user * filename)
        return getname_flags(filename, 0, NULL);
 }
 
+/*
+ * The "getname_kernel()" interface doesn't do pathnames longer
+ * than EMBEDDED_NAME_MAX. Deal with it - you're a kernel user.
+ */
+struct filename *
+getname_kernel(const char * filename)
+{
+       struct filename *result;
+       char *kname;
+       int len;
+
+       len = strlen(filename);
+       if (len >= EMBEDDED_NAME_MAX)
+               return ERR_PTR(-ENAMETOOLONG);
+
+       result = __getname();
+       if (unlikely(!result))
+               return ERR_PTR(-ENOMEM);
+
+       kname = (char *)result + sizeof(*result);
+       result->name = kname;
+       result->uptr = NULL;
+       result->aname = NULL;
+       result->separate = false;
+
+       strlcpy(kname, filename, EMBEDDED_NAME_MAX);
+       return result;
+}
+
 #ifdef CONFIG_AUDITSYSCALL
 void putname(struct filename *name)
 {

diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index fd8bf3219ef7bbc9af53a05e391215a978de1a19..b4a745d7d9a9005b99970afc0ad595fb0efe8e85 100644 (file)
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -115,7 +115,6 @@ extern int copy_strings_kernel(int argc, const char *const *argv,
 extern int prepare_bprm_creds(struct linux_binprm *bprm);
 extern void install_exec_creds(struct linux_binprm *bprm);
 extern void set_binfmt(struct linux_binfmt *new);
-extern void free_bprm(struct linux_binprm *);
 extern ssize_t read_code(struct file *, unsigned long, loff_t, size_t);
 
 #endif /* _LINUX_BINFMTS_H */

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 09f553c59813a2b2f88991ab0ab854880e07e574..d79678c188ad5a1831283b8a5051b30c5aa03ee4 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2079,6 +2079,7 @@ extern struct file * dentry_open(const struct path *, int, const struct cred *);
 extern int filp_close(struct file *, fl_owner_t id);
 
 extern struct filename *getname(const char __user *);
+extern struct filename *getname_kernel(const char *);
 
 enum {
        FILE_CREATED = 1,

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 68a0e84463a0eb86b273fe14b49bd9b01feab21f..a781dec1cd0b58d219427fdc71b37574ac972ec8 100644 (file)
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -128,6 +128,7 @@ struct bio_list;
 struct fs_struct;
 struct perf_event_context;
 struct blk_plug;
+struct filename;
 
 /*
  * List of flags we want to share for kernel threads,
@@ -2311,7 +2312,7 @@ extern void do_group_exit(int);
 extern int allow_signal(int);
 extern int disallow_signal(int);
 
-extern int do_execve(const char *,
+extern int do_execve(struct filename *,
                     const char __user * const __user *,
                     const char __user * const __user *);
 extern long do_fork(unsigned long, unsigned long, unsigned long, int __user *, int __user *);

diff --git a/init/main.c b/init/main.c
index 2fd9cef70ee8aa86261d8e2439b1c2a50f130925..eb03090cdced5aac82787cf3154c2430b7410924 100644 (file)
--- a/init/main.c
+++ b/init/main.c
@@ -812,7 +812,7 @@ void __init load_default_modules(void)
 static int run_init_process(const char *init_filename)
 {
        argv_init[0] = init_filename;
-       return do_execve(init_filename,
+       return do_execve(getname_kernel(init_filename),
                (const char __user *const __user *)argv_init,
                (const char __user *const __user *)envp_init);
 }

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 10176cd5956a7ccd9c3f34093bca928a0f5b0323..7aef2f4b6c644963fb33c320c212eb856df8c69a 100644 (file)
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1719,7 +1719,7 @@ void audit_putname(struct filename *name)
        struct audit_context *context = current->audit_context;
 
        BUG_ON(!context);
-       if (!context->in_syscall) {
+       if (!name->aname || !context->in_syscall) {
 #if AUDIT_DEBUG == 2
                printk(KERN_ERR "%s:%d(:%d): final_putname(%p)\n",
                       __FILE__, __LINE__, context->serial, name);

diff --git a/kernel/kmod.c b/kernel/kmod.c
index b086006c59e7c6957a51984a3ec101ea2db8525d..6b375af4958d1290c5c7a066903203802ee41d77 100644 (file)
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -239,7 +239,7 @@ static int ____call_usermodehelper(void *data)
 
        commit_creds(new);
 
-       retval = do_execve(sub_info->path,
+       retval = do_execve(getname_kernel(sub_info->path),
                           (const char __user *const __user *)sub_info->argv,
                           (const char __user *const __user *)sub_info->envp);
        if (!retval)