1
0
Fork 0
alistair23-linux/security
David Howells b4a1b4f504 KEYS: Fix race between read and revoke
This fixes CVE-2015-7550.

There's a race between keyctl_read() and keyctl_revoke().  If the revoke
happens between keyctl_read() checking the validity of a key and the key's
semaphore being taken, then the key type read method will see a revoked key.

This causes a problem for the user-defined key type because it assumes in
its read method that there will always be a payload in a non-revoked key
and doesn't check for a NULL pointer.

Fix this by making keyctl_read() check the validity of a key after taking
semaphore instead of before.

I think the bug was introduced with the original keyrings code.

This was discovered by a multithreaded test program generated by syzkaller
(http://github.com/google/syzkaller).  Here's a cleaned up version:

	#include <sys/types.h>
	#include <keyutils.h>
	#include <pthread.h>
	void *thr0(void *arg)
	{
		key_serial_t key = (unsigned long)arg;
		keyctl_revoke(key);
		return 0;
	}
	void *thr1(void *arg)
	{
		key_serial_t key = (unsigned long)arg;
		char buffer[16];
		keyctl_read(key, buffer, 16);
		return 0;
	}
	int main()
	{
		key_serial_t key = add_key("user", "%", "foo", 3, KEY_SPEC_USER_KEYRING);
		pthread_t th[5];
		pthread_create(&th[0], 0, thr0, (void *)(unsigned long)key);
		pthread_create(&th[1], 0, thr1, (void *)(unsigned long)key);
		pthread_create(&th[2], 0, thr0, (void *)(unsigned long)key);
		pthread_create(&th[3], 0, thr1, (void *)(unsigned long)key);
		pthread_join(th[0], 0);
		pthread_join(th[1], 0);
		pthread_join(th[2], 0);
		pthread_join(th[3], 0);
		return 0;
	}

Build as:

	cc -o keyctl-race keyctl-race.c -lkeyutils -lpthread

Run as:

	while keyctl-race; do :; done

as it may need several iterations to crash the kernel.  The crash can be
summarised as:

	BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
	IP: [<ffffffff81279b08>] user_read+0x56/0xa3
	...
	Call Trace:
	 [<ffffffff81276aa9>] keyctl_read_key+0xb6/0xd7
	 [<ffffffff81277815>] SyS_keyctl+0x83/0xe0
	 [<ffffffff815dbb97>] entry_SYSCALL_64_fastpath+0x12/0x6f

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: James Morris <james.l.morris@oracle.com>
2015-12-19 12:34:43 +11:00
..
apparmor apparmor: clarify CRYPTO dependency 2015-10-22 11:11:28 +11:00
integrity mm, page_alloc: rename __GFP_WAIT to __GFP_RECLAIM 2015-11-06 17:50:42 -08:00
keys KEYS: Fix race between read and revoke 2015-12-19 12:34:43 +11:00
selinux Merge branch 'upstream' of git://git.infradead.org/users/pcmoore/selinux into for-linus2 2015-11-26 15:04:19 +11:00
smack Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net 2015-11-10 18:11:41 -08:00
tomoyo LSM: Switch to lists of hooks 2015-05-12 15:00:41 +10:00
yama Adding YAMA hooks also when YAMA is not stacked. 2015-08-04 01:36:18 +10:00
Kconfig Yama: remove needless CONFIG_SECURITY_YAMA_STACKED 2015-07-28 13:18:19 +10:00
Makefile LSM: Switch to lists of hooks 2015-05-12 15:00:41 +10:00
commoncap.c capabilities: add a securebit to disable PR_CAP_AMBIENT_RAISE 2015-09-04 16:54:41 -07:00
device_cgroup.c security/device_cgroup: Fix RCU_LOCKDEP_WARN() condition 2015-09-03 18:13:10 -07:00
inode.c Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs 2015-07-04 19:36:06 -07:00
lsm_audit.c Merge branch 'next' of git://git.infradead.org/users/pcmoore/selinux into next 2015-08-15 13:29:57 +10:00
min_addr.c mmap_min_addr check CAP_SYS_RAWIO only for write 2010-04-23 08:56:31 +10:00
security.c Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security 2015-09-08 12:41:25 -07:00