From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from thejh.net ([37.221.195.125]:48199 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761378AbcIWUkv (ORCPT ); Fri, 23 Sep 2016 16:40:51 -0400 From: Jann Horn To: Alexander Viro , Roland McGrath , Oleg Nesterov , John Johansen , James Morris , "Serge E. Hallyn" , Paul Moore , Stephen Smalley , Eric Paris , Casey Schaufler , Kees Cook , Andrew Morton , Janis Danisevskis , Seth Forshee , "Eric . Biederman" , Thomas Gleixner , Benjamin LaHaise , Ben Hutchings , Andy Lutomirski , Linus Torvalds Cc: linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, security@kernel.org Subject: [PATCH v2 4/8] futex: don't leak robust_list pointer Date: Fri, 23 Sep 2016 22:40:34 +0200 Message-Id: <1474663238-22134-5-git-send-email-jann@thejh.net> In-Reply-To: <1474663238-22134-1-git-send-email-jann@thejh.net> References: <1474663238-22134-1-git-send-email-jann@thejh.net> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: This prevents an attacker from determining the robust_list or compat_robust_list userspace pointer of a process created by executing a setuid binary. Such an attack could be performed by racing get_robust_list() with a setuid execution. The impact of this issue is that an attacker could theoretically bypass ASLR when attacking setuid binaries. changed in v2: - only get_task_struct(p) if p!=NULL (Ben Hutchings) - move the -ESRCH return check Signed-off-by: Jann Horn --- kernel/futex.c | 30 +++++++++++++++++++++--------- kernel/futex_compat.c | 30 +++++++++++++++++++++--------- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/kernel/futex.c b/kernel/futex.c index 46cb3a3..773806e 100644 --- a/kernel/futex.c +++ b/kernel/futex.c @@ -3007,31 +3007,43 @@ SYSCALL_DEFINE3(get_robust_list, int, pid, if (!futex_cmpxchg_enabled) return -ENOSYS; - rcu_read_lock(); - - ret = -ESRCH; - if (!pid) + if (!pid) { p = current; - else { + get_task_struct(p); + } else { + rcu_read_lock(); p = find_task_by_vpid(pid); + /* pin the task to permit dropping the RCU read lock before + * acquiring the mutex + */ + if (p) + get_task_struct(p); + rcu_read_unlock(); if (!p) - goto err_unlock; + return -ESRCH; } + ret = mutex_lock_killable(&p->signal->cred_guard_light); + if (ret) + goto err_put; + ret = -EPERM; if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS)) goto err_unlock; head = p->robust_list; - rcu_read_unlock(); + + mutex_unlock(&p->signal->cred_guard_light); + put_task_struct(p); if (put_user(sizeof(*head), len_ptr)) return -EFAULT; return put_user(head, head_ptr); err_unlock: - rcu_read_unlock(); - + mutex_unlock(¤t->signal->cred_guard_light); +err_put: + put_task_struct(p); return ret; } diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c index 4ae3232..92c350f 100644 --- a/kernel/futex_compat.c +++ b/kernel/futex_compat.c @@ -143,31 +143,43 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid, if (!futex_cmpxchg_enabled) return -ENOSYS; - rcu_read_lock(); - - ret = -ESRCH; - if (!pid) + if (!pid) { p = current; - else { + get_task_struct(p); + } else { + rcu_read_lock(); p = find_task_by_vpid(pid); + /* pin the task to permit dropping the RCU read lock before + * acquiring the mutex + */ + if (p) + get_task_struct(p); + rcu_read_unlock(); if (!p) - goto err_unlock; + return -ESRCH; } + ret = mutex_lock_killable(&p->signal->cred_guard_light); + if (ret) + goto err_put; + ret = -EPERM; if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS)) goto err_unlock; head = p->compat_robust_list; - rcu_read_unlock(); + + mutex_unlock(&p->signal->cred_guard_light); + put_task_struct(p); if (put_user(sizeof(*head), len_ptr)) return -EFAULT; return put_user(ptr_to_compat(head), head_ptr); err_unlock: - rcu_read_unlock(); - + mutex_unlock(¤t->signal->cred_guard_light); +err_put: + put_task_struct(p); return ret; } -- 2.1.4