linux/security
David Howells 96ace77335 keys: fix race with concurrent install_user_keyrings()
commit 0da9dfdd2c upstream.

This fixes CVE-2013-1792.

There is a race in install_user_keyrings() that can cause a NULL pointer
dereference when called concurrently for the same user if the uid and
uid-session keyrings are not yet created.  It might be possible for an
unprivileged user to trigger this by calling keyctl() from userspace in
parallel immediately after logging in.

Assume that we have two threads both executing lookup_user_key(), both
looking for KEY_SPEC_USER_SESSION_KEYRING.

	THREAD A			THREAD B
	===============================	===============================
					==>call install_user_keyrings();
	if (!cred->user->session_keyring)
	==>call install_user_keyrings()
					...
					user->uid_keyring = uid_keyring;
	if (user->uid_keyring)
		return 0;
	<==
	key = cred->user->session_keyring [== NULL]
					user->session_keyring = session_keyring;
	atomic_inc(&key->usage); [oops]

At the point thread A dereferences cred->user->session_keyring, thread B
hasn't updated user->session_keyring yet, but thread A assumes it is
populated because install_user_keyrings() returned ok.

The race window is really small but can be exploited if, for example,
thread B is interrupted or preempted after initializing uid_keyring, but
before doing setting session_keyring.

This couldn't be reproduced on a stock kernel.  However, after placing
systemtap probe on 'user->session_keyring = session_keyring;' that
introduced some delay, the kernel could be crashed reliably.

Fix this by checking both pointers before deciding whether to return.
Alternatively, the test could be done away with entirely as it is checked
inside the mutex - but since the mutex is global, that may not be the best
way.

Signed-off-by: David Howells <dhowells@redhat.com>
Reported-by: Mateusz Guzik <mguzik@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2013-03-14 11:29:51 -07:00
..
apparmor lsm_audit: don't specify the audit pre/post callbacks in 'struct common_audit_data' 2012-04-03 09:49:59 -07:00
integrity evm: checking if removexattr is not a NULL 2013-01-27 20:47:43 -08:00
keys keys: fix race with concurrent install_user_keyrings() 2013-03-14 11:29:51 -07:00
selinux selinux: fix sel_netnode_insert() suspicious rcu dereference 2012-11-26 11:38:02 -08:00
smack Smack: move label list initialization 2012-04-18 12:02:28 +10:00
tomoyo usermodehelper: use UMH_WAIT_PROC consistently 2012-03-23 16:58:41 -07:00
yama Yama: handle 32-bit userspace prctl 2012-10-07 08:32:28 -07:00
capability.c security: create task_free security callback 2012-02-10 09:14:51 +11:00
commoncap.c security: fix compile error in commoncap.c 2012-04-19 12:56:39 +10:00
device_cgroup.c cgroup: remove cgroup_subsys argument from callbacks 2012-02-02 09:20:22 -08:00
inode.c securityfs: fix object creation races 2012-01-10 10:20:35 -05:00
Kconfig security: Yama LSM 2012-02-10 09:18:52 +11:00
lsm_audit.c lsm_audit: don't specify the audit pre/post callbacks in 'struct common_audit_data' 2012-04-03 09:49:59 -07:00
Makefile security: Yama LSM 2012-02-10 09:18:52 +11:00
min_addr.c mmap_min_addr check CAP_SYS_RAWIO only for write 2010-04-23 08:56:31 +10:00
security.c security: trim security.h 2012-02-14 10:45:42 +11:00