linux-toradex.git/security/keys, branch v4.1.20

KEYS: Fix keyring ref leak in join_session_keyring()

2016-01-23T04:54:17+00:00

commit 23567fd052a9abb6d67fe8e7a9ccdd9800a540f2 upstream.

This fixes CVE-2016-0728.

If a thread is asked to join as a session keyring the keyring that's already
set as its session, we leak a keyring reference.

This can be tested with the following program:

	#include 
	#include 
	#include 
	#include 

	int main(int argc, const char *argv[])
	{
		int i = 0;
		key_serial_t serial;

		serial = keyctl(KEYCTL_JOIN_SESSION_KEYRING,
				"leaked-keyring");
		if (serial < 0) {
			perror("keyctl");
			return -1;
		}

		if (keyctl(KEYCTL_SETPERM, serial,
			   KEY_POS_ALL | KEY_USR_ALL) < 0) {
			perror("keyctl");
			return -1;
		}

		for (i = 0; i < 100; i++) {
			serial = keyctl(KEYCTL_JOIN_SESSION_KEYRING,
					"leaked-keyring");
			if (serial < 0) {
				perror("keyctl");
				return -1;
			}
		}

		return 0;
	}

If, after the program has run, there something like the following line in
/proc/keys:

3f3d898f I--Q---   100 perm 3f3f0000     0     0 keyring   leaked-keyring: empty

with a usage count of 100 * the number of times the program has been run,
then the kernel is malfunctioning.  If leaked-keyring has zero usages or
has been garbage collected, then the problem is fixed.

Reported-by: Yevgeny Pats 
Signed-off-by: David Howells 
Acked-by: Don Zickus 
Acked-by: Prarit Bhargava 
Acked-by: Jarod Wilson 
Signed-off-by: James Morris 
Signed-off-by: Greg Kroah-Hartman

KEYS: Fix race between read and revoke

2016-01-23T04:54:17+00:00

commit b4a1b4f5047e4f54e194681125c74c0aa64d637d upstream.

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 
	#include 
	#include 
	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: [] user_read+0x56/0xa3
	...
	Call Trace:
	 [] keyctl_read_key+0xb6/0xd7
	 [] SyS_keyctl+0x83/0xe0
	 [] entry_SYSCALL_64_fastpath+0x12/0x6f

Reported-by: Dmitry Vyukov 
Signed-off-by: David Howells 
Tested-by: Dmitry Vyukov 
Signed-off-by: James Morris 
Signed-off-by: Greg Kroah-Hartman

KEYS: Fix crash when attempt to garbage collect an uninstantiated keyring

2016-01-23T04:54:16+00:00

commit f05819df10d7b09f6d1eb6f8534a8f68e5a4fe61 upstream.

The following sequence of commands:

    i=`keyctl add user a a @s`
    keyctl request2 keyring foo bar @t
    keyctl unlink $i @s

tries to invoke an upcall to instantiate a keyring if one doesn't already
exist by that name within the user's keyring set.  However, if the upcall
fails, the code sets keyring->type_data.reject_error to -ENOKEY or some
other error code.  When the key is garbage collected, the key destroy
function is called unconditionally and keyring_destroy() uses list_empty()
on keyring->type_data.link - which is in a union with reject_error.
Subsequently, the kernel tries to unlink the keyring from the keyring names
list - which oopses like this:

	BUG: unable to handle kernel paging request at 00000000ffffff8a
	IP: [] keyring_destroy+0x3d/0x88
	...
	Workqueue: events key_garbage_collector
	...
	RIP: 0010:[] keyring_destroy+0x3d/0x88
	RSP: 0018:ffff88003e2f3d30  EFLAGS: 00010203
	RAX: 00000000ffffff82 RBX: ffff88003bf1a900 RCX: 0000000000000000
	RDX: 0000000000000000 RSI: 000000003bfc6901 RDI: ffffffff81a73a40
	RBP: ffff88003e2f3d38 R08: 0000000000000152 R09: 0000000000000000
	R10: ffff88003e2f3c18 R11: 000000000000865b R12: ffff88003bf1a900
	R13: 0000000000000000 R14: ffff88003bf1a908 R15: ffff88003e2f4000
	...
	CR2: 00000000ffffff8a CR3: 000000003e3ec000 CR4: 00000000000006f0
	...
	Call Trace:
	 [] key_gc_unused_keys.constprop.1+0x5d/0x10f
	 [] key_garbage_collector+0x1fa/0x351
	 [] process_one_work+0x28e/0x547
	 [] worker_thread+0x26e/0x361
	 [] ? rescuer_thread+0x2a8/0x2a8
	 [] kthread+0xf3/0xfb
	 [] ? kthread_create_on_node+0x1c2/0x1c2
	 [] ret_from_fork+0x3f/0x70
	 [] ? kthread_create_on_node+0x1c2/0x1c2

Note the value in RAX.  This is a 32-bit representation of -ENOKEY.

The solution is to only call ->destroy() if the key was successfully
instantiated.

Reported-by: Dmitry Vyukov 
Signed-off-by: David Howells 
Tested-by: Dmitry Vyukov 
Signed-off-by: Greg Kroah-Hartman

KEYS: Fix race between key destruction and finding a keyring by name

2016-01-23T04:54:16+00:00

commit 94c4554ba07adbdde396748ee7ae01e86cf2d8d7 upstream.

There appears to be a race between:

 (1) key_gc_unused_keys() which frees key->security and then calls
     keyring_destroy() to unlink the name from the name list

 (2) find_keyring_by_name() which calls key_permission(), thus accessing
     key->security, on a key before checking to see whether the key usage is 0
     (ie. the key is dead and might be cleaned up).

Fix this by calling ->destroy() before cleaning up the core key data -
including key->security.

Reported-by: Petr Matousek 
Signed-off-by: David Howells 
Signed-off-by: Greg Kroah-Hartman

KEYS: ensure we free the assoc array edit if edit is valid

2015-08-03T16:29:11+00:00

commit ca4da5dd1f99fe9c59f1709fb43e818b18ad20e0 upstream.

__key_link_end is not freeing the associated array edit structure
and this leads to a 512 byte memory leak each time an identical
existing key is added with add_key().

The reason the add_key() system call returns okay is that
key_create_or_update() calls __key_link_begin() before checking to see
whether it can update a key directly rather than adding/replacing - which
it turns out it can.  Thus __key_link() is not called through
__key_instantiate_and_link() and __key_link_end() must cancel the edit.

CVE-2015-1333

Signed-off-by: Colin Ian King 
Signed-off-by: David Howells 
Signed-off-by: James Morris 
Signed-off-by: Greg Kroah-Hartman

switch keyctl_instantiate_key_common() to iov_iter

2015-04-12T02:27:12+00:00

Signed-off-by: Al Viro

Don't leak a key reference if request_key() tries to use a revoked keyring

2015-02-16T02:45:16+00:00

If a request_key() call to allocate and fill out a key attempts to insert the
key structure into a revoked keyring, the key will leak, using memory and part
of the user's key quota until the system reboots. This is from a failure of
construct_alloc_key() to decrement the key's reference count after the attempt
to insert into the requested keyring is rejected.

key_put() needs to be called in the link_prealloc_failed callpath to ensure
the unused key is released.

Signed-off-by: David Jeffery 
Signed-off-by: David Howells 
Signed-off-by: James Morris

KEYS: Make /proc/keys unconditional if CONFIG_KEYS=y

2015-01-22T22:34:32+00:00

Now that /proc/keys is used by libkeyutils to look up a key by type and
description, we should make it unconditional and remove
CONFIG_DEBUG_PROC_KEYS.

Reported-by: Jiri Kosina 
Signed-off-by: David Howells 
Tested-by: Jiri Kosina

KEYS: close race between key lookup and freeing

2015-01-05T15:58:01+00:00

When a key is being garbage collected, it's key->user would get put before
the ->destroy() callback is called, where the key is removed from it's
respective tracking structures.

This leaves a key hanging in a semi-invalid state which leaves a window open
for a different task to try an access key->user. An example is
find_keyring_by_name() which would dereference key->user for a key that is
in the process of being garbage collected (where key->user was freed but
->destroy() wasn't called yet - so it's still present in the linked list).

This would cause either a panic, or corrupt memory.

Fixes CVE-2014-9529.

Signed-off-by: Sasha Levin 
Signed-off-by: David Howells

KEYS: remove a bogus NULL check

2014-12-16T07:05:20+00:00

We already checked if "desc" was NULL at the beginning of the function
and we've dereferenced it so this causes a static checker warning.

Signed-off-by: Dan Carpenter 
Signed-off-by: David Howells 
Signed-off-by: James Morris