linux-kernel - [PATCH 1/8] kernel/exit.c: make sure current's nsproxy != NULL while checking caps

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <1432209222-8479-2-git-send-email-l.pawelczyk@samsung.com>
Date:	Thu, 21 May 2015 13:53:35 +0200
From:	Lukasz Pawelczyk <l.pawelczyk@...sung.com>
To:	"David S. Miller" <davem@...emloft.net>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	"Kirill A. Shutemov" <kirill@...temov.name>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Alexey Dobriyan <adobriyan@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andy Lutomirski <luto@...capital.net>,
	Casey Schaufler <casey@...aufler-ca.com>,
	Christoph Hellwig <hch@....de>,
	David Howells <dhowells@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Fabian Frederick <fabf@...net.be>,
	Greg KH <gregkh@...uxfoundation.org>,
	Ingo Molnar <mingo@...nel.org>,
	Ionut Alexa <ionut.m.alexa@...il.com>,
	James Morris <james.l.morris@...cle.com>,
	Jeff Layton <jlayton@...marydata.com>,
	Joe Perches <joe@...ches.com>,
	Jonathan Corbet <corbet@....net>,
	Kees Cook <keescook@...omium.org>,
	Lukasz Pawelczyk <l.pawelczyk@...sung.com>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Michal Hocko <mhocko@...e.cz>,
	Miklos Szeredi <miklos@...redi.hu>,
	Nick Kralevich <nnk@...gle.com>,
	Oleg Nesterov <oleg@...hat.com>,
	Paul Moore <pmoore@...hat.com>,
	Peter Hurley <peter@...leysoftware.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Rik van Riel <riel@...hat.com>,
	Serge Hallyn <serge.hallyn@...onical.com>,
	Stephen Smalley <sds@...ho.nsa.gov>, Tejun Heo <tj@...nel.org>,
	Zefan Li <lizefan@...wei.com>,
	Rafal Krypa <r.krypa@...sung.com>, linux-doc@...r.kernel.org,
	linux-api@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	containers@...ts.linux-foundation.org
Cc:	Lukasz Pawelczyk <havner@...il.com>
Subject: [PATCH 1/8] kernel/exit.c: make sure current's nsproxy != NULL while
 checking caps

There is a rare case where current's nsproxy might be NULL but we are
required to check for credentials and capabilities. It sometimes happens
during an exit_group() syscall while destroying user's session (logging
out).

My understanding is that while we have to lock the task to get task's
nsproxy and check whether it's NULL, for the 'current' we don't have to
and it's expected not to be NULL. There is a code in the kernel
currently that does current->nsproxy->user_ns without any checks.
And include/linux/nsproxy.h confirms that:

2. when accessing (i.e. reading) current task's namespaces - no
   precautions should be taken - just dereference the pointers

There seem to be no crash currently because of this, but with accessing
nsproxy from LSM hooks there is. This is the backtrace:

0  smk_tskacc (task=0xffff88003b0b92e0, obj_known=0x2 <irq_stack_union+2>, mode=2, a=0xffff88003be53dd8) at security/smack/smack_access.c:261
1  0xffffffff8130e2aa in smk_curacc (obj_known=<optimized out>, mode=<optimized out>, a=<optimized out>) at security/smack/smack_access.c:318
2  0xffffffff8130a50d in smack_task_kill (p=0xffff88003b0b92e0, info=<optimized out>, sig=<optimized out>, secid=<optimized out>) at security/smack/smack_lsm.c:2071
3  0xffffffff812ea4f6 in security_task_kill (p=<optimized out>, info=<optimized out>, sig=<optimized out>, secid=<optimized out>) at security/security.c:952
4  0xffffffff8109ac80 in check_kill_permission (sig=15, info=0x0 <irq_stack_union>, t=0xffff88003b0b8000) at kernel/signal.c:796
5  0xffffffff8109d3ab in group_send_sig_info (sig=15, info=0x0 <irq_stack_union>, p=0xffff88003b0b8000) at kernel/signal.c:1296
6  0xffffffff8108e527 in forget_original_parent (father=<optimized out>) at kernel/exit.c:575
7  exit_notify (group_dead=<optimized out>, tsk=<optimized out>) at kernel/exit.c:606
8  do_exit (code=<optimized out>) at kernel/exit.c:775
9  0xffffffff8108ec0f in do_group_exit (exit_code=0) at kernel/exit.c:891
10 0xffffffff8108ec84 in SYSC_exit_group (error_code=<optimized out>) at kernel/exit.c:902
11 SyS_exit_group (error_code=<optimized out>) at kernel/exit.c:900

This backtrace clearly shows that there is an LSM hook task_kill() that
happens during an exit_group() syscall and that this happens after
exit_task_namespaces(). LSM hooks with namespaces might need nsproxy to
be able to check for capabilities. At this point this is impossible. The
current's nsproxy is already NULL/destroyed.

This is the case because exit_task_namespaces() is called before the
exit_notify() where all of the above happens. This patch changes their
order.

Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@...sung.com>
---
 kernel/exit.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 22fcc05..da1bb18 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -742,7 +742,6 @@ void do_exit(long code)
 	exit_fs(tsk);
 	if (group_dead)
 		disassociate_ctty(1);
-	exit_task_namespaces(tsk);
 	exit_task_work(tsk);
 	exit_thread();
 
@@ -763,6 +762,13 @@ void do_exit(long code)
 
 	TASKS_RCU(tasks_rcu_i = __srcu_read_lock(&tasks_rcu_exit_srcu));
 	exit_notify(tsk, group_dead);
+
+	/*
+	 * This should be after all things that potentially require
+	 * process's namespaces (e.g. capability checks).
+	 */
+	exit_task_namespaces(tsk);
+
 	proc_exit_connector(tsk);
 #ifdef CONFIG_NUMA
 	task_lock(tsk);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/