Commit fd797512 authored by Jann Horn's avatar Jann Horn Committed by Jiri Slaby

fs/coredump: prevent fsuid=0 dumps into user-controlled directories

commit 378c6520 upstream.

This commit fixes the following security hole affecting systems where
all of the following conditions are fulfilled:

 - The fs.suid_dumpable sysctl is set to 2.
 - The kernel.core_pattern sysctl's value starts with "/". (Systems
   where kernel.core_pattern starts with "|/" are not affected.)
 - Unprivileged user namespace creation is permitted. (This is
   true on Linux >=3.8, but some distributions disallow it by
   default using a distro patch.)

Under these conditions, if a program executes under secure exec rules,
causing it to run with the SUID_DUMP_ROOT flag, then unshares its user
namespace, changes its root directory and crashes, the coredump will be
written using fsuid=0 and a path derived from kernel.core_pattern - but
this path is interpreted relative to the root directory of the process,
allowing the attacker to control where a coredump will be written with
root privileges.

To fix the security issue, always interpret core_pattern for dumps that
are written under SUID_DUMP_ROOT relative to the root directory of init.
Signed-off-by: default avatarJann Horn <jann@thejh.net>
Acked-by: default avatarKees Cook <keescook@chromium.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
parent cc2a8c04
...@@ -133,7 +133,7 @@ void mconsole_proc(struct mc_request *req) ...@@ -133,7 +133,7 @@ void mconsole_proc(struct mc_request *req)
ptr += strlen("proc"); ptr += strlen("proc");
ptr = skip_spaces(ptr); ptr = skip_spaces(ptr);
file = file_open_root(mnt->mnt_root, mnt, ptr, O_RDONLY); file = file_open_root(mnt->mnt_root, mnt, ptr, O_RDONLY, 0);
if (IS_ERR(file)) { if (IS_ERR(file)) {
mconsole_reply(req, "Failed to open file", 1, 0); mconsole_reply(req, "Failed to open file", 1, 0);
printk(KERN_ERR "open /proc/%s: %ld\n", ptr, PTR_ERR(file)); printk(KERN_ERR "open /proc/%s: %ld\n", ptr, PTR_ERR(file));
......
...@@ -32,6 +32,9 @@ ...@@ -32,6 +32,9 @@
#include <linux/pipe_fs_i.h> #include <linux/pipe_fs_i.h>
#include <linux/oom.h> #include <linux/oom.h>
#include <linux/compat.h> #include <linux/compat.h>
#include <linux/sched.h>
#include <linux/fs.h>
#include <linux/path.h>
#include <asm/uaccess.h> #include <asm/uaccess.h>
#include <asm/mmu_context.h> #include <asm/mmu_context.h>
...@@ -614,6 +617,8 @@ void do_coredump(siginfo_t *siginfo) ...@@ -614,6 +617,8 @@ void do_coredump(siginfo_t *siginfo)
} }
} else { } else {
struct inode *inode; struct inode *inode;
int open_flags = O_CREAT | O_RDWR | O_NOFOLLOW |
O_LARGEFILE | O_EXCL;
if (cprm.limit < binfmt->min_coredump) if (cprm.limit < binfmt->min_coredump)
goto fail_unlock; goto fail_unlock;
...@@ -652,10 +657,27 @@ void do_coredump(siginfo_t *siginfo) ...@@ -652,10 +657,27 @@ void do_coredump(siginfo_t *siginfo)
* what matters is that at least one of the two processes * what matters is that at least one of the two processes
* writes its coredump successfully, not which one. * writes its coredump successfully, not which one.
*/ */
cprm.file = filp_open(cn.corename, if (need_suid_safe) {
O_CREAT | 2 | O_NOFOLLOW | /*
O_LARGEFILE | O_EXCL, * Using user namespaces, normal user tasks can change
0600); * their current->fs->root to point to arbitrary
* directories. Since the intention of the "only dump
* with a fully qualified path" rule is to control where
* coredumps may be placed using root privileges,
* current->fs->root must not be used. Instead, use the
* root directory of init_task.
*/
struct path root;
task_lock(&init_task);
get_fs_root(init_task.fs, &root);
task_unlock(&init_task);
cprm.file = file_open_root(root.dentry, root.mnt,
cn.corename, open_flags, 0600);
path_put(&root);
} else {
cprm.file = filp_open(cn.corename, open_flags, 0600);
}
if (IS_ERR(cprm.file)) if (IS_ERR(cprm.file))
goto fail_unlock; goto fail_unlock;
......
...@@ -228,7 +228,7 @@ long do_handle_open(int mountdirfd, ...@@ -228,7 +228,7 @@ long do_handle_open(int mountdirfd,
path_put(&path); path_put(&path);
return fd; return fd;
} }
file = file_open_root(path.dentry, path.mnt, "", open_flag); file = file_open_root(path.dentry, path.mnt, "", open_flag, 0);
if (IS_ERR(file)) { if (IS_ERR(file)) {
put_unused_fd(fd); put_unused_fd(fd);
retval = PTR_ERR(file); retval = PTR_ERR(file);
......
...@@ -923,14 +923,12 @@ struct file *filp_open(const char *filename, int flags, umode_t mode) ...@@ -923,14 +923,12 @@ struct file *filp_open(const char *filename, int flags, umode_t mode)
EXPORT_SYMBOL(filp_open); EXPORT_SYMBOL(filp_open);
struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt, struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt,
const char *filename, int flags) const char *filename, int flags, umode_t mode)
{ {
struct open_flags op; struct open_flags op;
int err = build_open_flags(flags, 0, &op); int err = build_open_flags(flags, mode, &op);
if (err) if (err)
return ERR_PTR(err); return ERR_PTR(err);
if (flags & O_CREAT)
return ERR_PTR(-EINVAL);
if (!filename && (flags & O_DIRECTORY)) if (!filename && (flags & O_DIRECTORY))
if (!dentry->d_inode->i_op->lookup) if (!dentry->d_inode->i_op->lookup)
return ERR_PTR(-ENOTDIR); return ERR_PTR(-ENOTDIR);
......
...@@ -2005,7 +2005,7 @@ extern long do_sys_open(int dfd, const char __user *filename, int flags, ...@@ -2005,7 +2005,7 @@ extern long do_sys_open(int dfd, const char __user *filename, int flags,
extern struct file *file_open_name(struct filename *, int, umode_t); extern struct file *file_open_name(struct filename *, int, umode_t);
extern struct file *filp_open(const char *, int, umode_t); extern struct file *filp_open(const char *, int, umode_t);
extern struct file *file_open_root(struct dentry *, struct vfsmount *, extern struct file *file_open_root(struct dentry *, struct vfsmount *,
const char *, int); const char *, int, umode_t);
extern struct file * dentry_open(const struct path *, int, const struct cred *); extern struct file * dentry_open(const struct path *, int, const struct cred *);
extern int filp_close(struct file *, fl_owner_t id); extern int filp_close(struct file *, fl_owner_t id);
......
...@@ -1320,7 +1320,7 @@ static ssize_t binary_sysctl(const int *name, int nlen, ...@@ -1320,7 +1320,7 @@ static ssize_t binary_sysctl(const int *name, int nlen,
} }
mnt = task_active_pid_ns(current)->proc_mnt; mnt = task_active_pid_ns(current)->proc_mnt;
file = file_open_root(mnt->mnt_root, mnt, pathname, flags); file = file_open_root(mnt->mnt_root, mnt, pathname, flags, 0);
result = PTR_ERR(file); result = PTR_ERR(file);
if (IS_ERR(file)) if (IS_ERR(file))
goto out_putname; goto out_putname;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment