3.2.93-rc1 review patch. If anyone has any objections, please let me know.
------------------ From: Jann Horn <[email protected]> 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). [[email protected]: fix warning] Signed-off-by: Jann Horn <[email protected]> Acked-by: Kees Cook <[email protected]> Cc: Casey Schaufler <[email protected]> Cc: Oleg Nesterov <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: James Morris <[email protected]> Cc: "Serge E. Hallyn" <[email protected]> Cc: Andy Shevchenko <[email protected]> Cc: Andy Lutomirski <[email protected]> Cc: Al Viro <[email protected]> Cc: "Eric W. Biederman" <[email protected]> Cc: Willy Tarreau <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> [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 <[email protected]> --- --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -380,7 +380,7 @@ static int do_task_stat(struct seq_file 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); --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -216,7 +216,7 @@ static struct mm_struct *mm_access(struc 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_st 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 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 */ 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, 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_ 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; } --- a/fs/proc/namespaces.c +++ b/fs/proc/namespaces.c @@ -91,7 +91,7 @@ static int proc_ns_dir_readdir(struct fi 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 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]; --- a/include/linux/ptrace.h +++ b/include/linux/ptrace.h @@ -130,9 +130,31 @@ extern void __ptrace_unlink(struct task_ 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) --- 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; --- a/kernel/futex.c +++ b/kernel/futex.c @@ -2627,7 +2627,7 @@ SYSCALL_DEFINE3(get_robust_list, int, pi } 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; --- a/kernel/futex_compat.c +++ b/kernel/futex_compat.c @@ -154,7 +154,7 @@ compat_sys_get_robust_list(int pid, comp } 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; --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -220,6 +220,14 @@ int ptrace_check_attach(struct task_stru 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_stru * 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_str 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; --- 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 } 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; --- a/security/commoncap.c +++ b/security/commoncap.c @@ -141,12 +141,17 @@ int cap_ptrace_access_check(struct task_ { 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;
