From 1c8d42255f4c55f9550f72bbcda758aeff3fd7c2 Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Wed, 20 Jan 2016 15:00:04 -0800 Subject: [PATCH] ptrace: use fsuid, fsgid, effective creds for fs access checks commit caaee6234d05a58c5b4d05e7bf766131b810a657 upstream. By checking the effective credentials instead of the real UID / permitted capabilities, ensure that the calling process actually intended to use its credentials. To ensure that all ptrace checks use the correct caller credentials (e.g. in case out-of-tree code or newly added code omits the PTRACE_MODE_*CREDS flag), use two new flags and require one of them to be set. The problem was that when a privileged task had temporarily dropped its privileges, e.g. by calling setreuid(0, user_uid), with the intent to perform following syscalls with the credentials of a user, it still passed ptrace access checks that the user would not be able to pass. While an attacker should not be able to convince the privileged task to perform a ptrace() syscall, this is a problem because the ptrace access check is reused for things in procfs. In particular, the following somewhat interesting procfs entries only rely on ptrace access checks: /proc/$pid/stat - uses the check for determining whether pointers should be visible, useful for bypassing ASLR /proc/$pid/maps - also useful for bypassing ASLR /proc/$pid/cwd - useful for gaining access to restricted directories that contain files with lax permissions, e.g. in this scenario: lrwxrwxrwx root root /proc/13020/cwd -> /root/foobar drwx------ root root /root drwxr-xr-x root root /root/foobar -rw-r--r-- root root /root/foobar/secret Therefore, on a system where a root-owned mode 6755 binary changes its effective credentials as described and then dumps a user-specified file, this could be used by an attacker to reveal the memory layout of root's processes or reveal the contents of files he is not allowed to access (through /proc/$pid/cwd). [akpm@linux-foundation.org: fix warning] Signed-off-by: Jann Horn Acked-by: Kees Cook Cc: Casey Schaufler Cc: Oleg Nesterov Cc: Ingo Molnar Cc: James Morris Cc: "Serge E. Hallyn" Cc: Andy Shevchenko Cc: Andy Lutomirski Cc: Al Viro Cc: "Eric W. Biederman" Cc: Willy Tarreau Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Signed-off-by: Greg Kroah-Hartman [bwh: Backported to 3.2: - Drop changes to kcmp, procfs map_files, procfs has_pid_permissions() - Keep using uid_t, gid_t and == operator for IDs - Adjust context] Signed-off-by: Ben Hutchings --- fs/proc/array.c | 2 +- fs/proc/base.c | 12 ++++++------ fs/proc/namespaces.c | 4 ++-- include/linux/ptrace.h | 24 +++++++++++++++++++++++- kernel/events/core.c | 2 +- kernel/futex.c | 2 +- kernel/futex_compat.c | 2 +- kernel/ptrace.c | 39 +++++++++++++++++++++++++++++++-------- mm/process_vm_access.c | 2 +- security/commoncap.c | 7 ++++++- 10 files changed, 73 insertions(+), 23 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index 439b5a1048c9a..b83dcbb83d3fe 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -380,7 +380,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns, state = *get_task_state(task); vsize = eip = esp = 0; - permitted = ptrace_may_access(task, PTRACE_MODE_READ); + permitted = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS); mm = get_task_mm(task); if (mm) { vsize = task_vsize(mm); diff --git a/fs/proc/base.c b/fs/proc/base.c index 2c38a3e4fa51d..9bc4643054bfa 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -216,7 +216,7 @@ static struct mm_struct *mm_access(struct task_struct *task, unsigned int mode) struct mm_struct *mm_for_maps(struct task_struct *task) { - return mm_access(task, PTRACE_MODE_READ); + return mm_access(task, PTRACE_MODE_READ_FSCREDS); } static int proc_pid_cmdline(struct task_struct *task, char * buffer) @@ -288,7 +288,7 @@ static int proc_pid_wchan(struct task_struct *task, char *buffer) wchan = get_wchan(task); if (lookup_symbol_name(wchan, symname) < 0) - if (!ptrace_may_access(task, PTRACE_MODE_READ)) + if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) return 0; else return sprintf(buffer, "%lu", wchan); @@ -302,7 +302,7 @@ static int lock_trace(struct task_struct *task) int err = mutex_lock_killable(&task->signal->cred_guard_mutex); if (err) return err; - if (!ptrace_may_access(task, PTRACE_MODE_ATTACH)) { + if (!ptrace_may_access(task, PTRACE_MODE_ATTACH_FSCREDS)) { mutex_unlock(&task->signal->cred_guard_mutex); return -EPERM; } @@ -544,7 +544,7 @@ static int proc_fd_access_allowed(struct inode *inode) */ task = get_proc_task(inode); if (task) { - allowed = ptrace_may_access(task, PTRACE_MODE_READ); + allowed = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS); put_task_struct(task); } return allowed; @@ -769,7 +769,7 @@ static int mem_open(struct inode* inode, struct file* file) if (!task) return -ESRCH; - mm = mm_access(task, PTRACE_MODE_ATTACH); + mm = mm_access(task, PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS); put_task_struct(task); if (IS_ERR(mm)) @@ -2627,7 +2627,7 @@ static int do_io_accounting(struct task_struct *task, char *buffer, int whole) if (result) return result; - if (!ptrace_may_access(task, PTRACE_MODE_READ)) { + if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) { result = -EACCES; goto out_unlock; } diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c index d6c078ea14898..50de28b0a6709 100644 --- a/fs/proc/namespaces.c +++ b/fs/proc/namespaces.c @@ -91,7 +91,7 @@ static int proc_ns_dir_readdir(struct file *filp, void *dirent, goto out_no_task; ret = -EPERM; - if (!ptrace_may_access(task, PTRACE_MODE_READ)) + if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) goto out; ret = 0; @@ -154,7 +154,7 @@ static struct dentry *proc_ns_dir_lookup(struct inode *dir, goto out_no_task; error = ERR_PTR(-EPERM); - if (!ptrace_may_access(task, PTRACE_MODE_READ)) + if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) goto out; last = &ns_entries[ARRAY_SIZE(ns_entries) - 1]; diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h index e49240ba83466..953193651aa88 100644 --- a/include/linux/ptrace.h +++ b/include/linux/ptrace.h @@ -130,9 +130,31 @@ extern void __ptrace_unlink(struct task_struct *child); extern void exit_ptrace(struct task_struct *tracer); #define PTRACE_MODE_READ 1 #define PTRACE_MODE_ATTACH 2 +#define PTRACE_MODE_FSCREDS 0x08 +#define PTRACE_MODE_REALCREDS 0x10 + +/* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */ +#define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS) +#define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS) +#define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS) +#define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS) + /* Returns 0 on success, -errno on denial. */ extern int __ptrace_may_access(struct task_struct *task, unsigned int mode); -/* Returns true on success, false on denial. */ +/** + * ptrace_may_access - check whether the caller is permitted to access + * a target task. + * @task: target task + * @mode: selects type of access and caller credentials + * + * Returns true on success, false on denial. + * + * One of the flags PTRACE_MODE_FSCREDS and PTRACE_MODE_REALCREDS must + * be set in @mode to specify whether the access was requested through + * a filesystem syscall (should use effective capabilities and fsuid + * of the caller) or through an explicit syscall such as + * process_vm_writev or ptrace (and should use the real credentials). + */ extern bool ptrace_may_access(struct task_struct *task, unsigned int mode); static inline int ptrace_reparented(struct task_struct *child) diff --git a/kernel/events/core.c b/kernel/events/core.c index cd4f88a39fe10..bc94278e0b455 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -3004,7 +3004,7 @@ find_lively_task_by_vpid(pid_t vpid) /* Reuse ptrace permission checks for now. */ err = -EACCES; - if (!ptrace_may_access(task, PTRACE_MODE_READ)) + if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)) goto errout; return task; diff --git a/kernel/futex.c b/kernel/futex.c index bbd360110079d..3854e754f6411 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -2627,7 +2627,7 @@ SYSCALL_DEFINE3(get_robust_list, int, pid, } ret = -EPERM; - if (!ptrace_may_access(p, PTRACE_MODE_READ)) + if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS)) goto err_unlock; head = p->robust_list; diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c index a9642d5286302..e07b64b465fc3 100644 --- a/kernel/futex_compat.c +++ b/kernel/futex_compat.c @@ -154,7 +154,7 @@ compat_sys_get_robust_list(int pid, compat_uptr_t __user *head_ptr, } ret = -EPERM; - if (!ptrace_may_access(p, PTRACE_MODE_READ)) + if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS)) goto err_unlock; head = p->compat_robust_list; diff --git a/kernel/ptrace.c b/kernel/ptrace.c index fd3909b871887..b50cded5b80e1 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -220,6 +220,14 @@ int ptrace_check_attach(struct task_struct *child, bool ignore_state) int __ptrace_may_access(struct task_struct *task, unsigned int mode) { const struct cred *cred = current_cred(), *tcred; + int dumpable = 0; + uid_t caller_uid; + gid_t caller_gid; + + if (!(mode & PTRACE_MODE_FSCREDS) == !(mode & PTRACE_MODE_REALCREDS)) { + WARN(1, "denying ptrace access check without PTRACE_MODE_*CREDS\n"); + return -EPERM; + } /* May we inspect the given task? * This check is used both for attaching with ptrace @@ -229,19 +237,34 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode) * because setting up the necessary parent/child relationship * or halting the specified task is impossible. */ - int dumpable = 0; + /* Don't let security modules deny introspection */ if (same_thread_group(task, current)) return 0; rcu_read_lock(); + if (mode & PTRACE_MODE_FSCREDS) { + caller_uid = cred->fsuid; + caller_gid = cred->fsgid; + } else { + /* + * Using the euid would make more sense here, but something + * in userland might rely on the old behavior, and this + * shouldn't be a security problem since + * PTRACE_MODE_REALCREDS implies that the caller explicitly + * used a syscall that requests access to another process + * (and not a filesystem syscall to procfs). + */ + caller_uid = cred->uid; + caller_gid = cred->gid; + } tcred = __task_cred(task); if (cred->user->user_ns == tcred->user->user_ns && - (cred->uid == tcred->euid && - cred->uid == tcred->suid && - cred->uid == tcred->uid && - cred->gid == tcred->egid && - cred->gid == tcred->sgid && - cred->gid == tcred->gid)) + (caller_uid == tcred->euid && + caller_uid == tcred->suid && + caller_uid == tcred->uid && + caller_gid == tcred->egid && + caller_gid == tcred->sgid && + caller_gid == tcred->gid)) goto ok; if (ns_capable(tcred->user->user_ns, CAP_SYS_PTRACE)) goto ok; @@ -308,7 +331,7 @@ static int ptrace_attach(struct task_struct *task, long request, goto out; task_lock(task); - retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH); + retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS); task_unlock(task); if (retval) goto unlock_creds; diff --git a/mm/process_vm_access.c b/mm/process_vm_access.c index 70e814a4d0b87..88c35c9e4407f 100644 --- a/mm/process_vm_access.c +++ b/mm/process_vm_access.c @@ -299,7 +299,7 @@ static ssize_t process_vm_rw_core(pid_t pid, const struct iovec *lvec, } task_lock(task); - if (__ptrace_may_access(task, PTRACE_MODE_ATTACH)) { + if (__ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS)) { task_unlock(task); rc = -EPERM; goto put_task_struct; diff --git a/security/commoncap.c b/security/commoncap.c index 12440ee03c31c..4f338aba1ddb7 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -141,12 +141,17 @@ int cap_ptrace_access_check(struct task_struct *child, unsigned int mode) { int ret = 0; const struct cred *cred, *child_cred; + const kernel_cap_t *caller_caps; rcu_read_lock(); cred = current_cred(); child_cred = __task_cred(child); + if (mode & PTRACE_MODE_FSCREDS) + caller_caps = &cred->cap_effective; + else + caller_caps = &cred->cap_permitted; if (cred->user->user_ns == child_cred->user->user_ns && - cap_issubset(child_cred->cap_permitted, cred->cap_permitted)) + cap_issubset(child_cred->cap_permitted, *caller_caps)) goto out; if (ns_capable(child_cred->user->user_ns, CAP_SYS_PTRACE)) goto out; -- 2.47.2