Skip to content

L
linux
Switch branch/tag
  1. 29 Jan, 2016 35 commits
  3. 23 Jan, 2016 5 commits
    • Greg Kroah-Hartman's avatar
      Linux 3.14.59 · e9977508
      Greg Kroah-Hartman authored
      e9977508
    • Yevgeny Pats's avatar
      KEYS: Fix keyring ref leak in join_session_keyring() · 2e647bca
      Yevgeny Pats authored 
      commit 23567fd0 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 <stddef.h>
	#include <stdio.h>
	#include <sys/types.h>
	#include <keyutils.h>

	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: default avatarYevgeny Pats <yevgeny@perception-point.io>
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Acked-by: default avatarDon Zickus <dzickus@redhat.com>
Acked-by: default avatarPrarit Bhargava <prarit@redhat.com>
Acked-by: default avatarJarod Wilson <jarod@redhat.com>
Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      2e647bca
    • David Howells's avatar
      KEYS: Fix race between read and revoke · 92264cc9
      David Howells authored 
      commit b4a1b4f5 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 <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: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Tested-by: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      92264cc9
    • David Howells's avatar
      KEYS: Fix crash when attempt to garbage collect an uninstantiated keyring · aad1f1b8
      David Howells authored 
      commit f05819df 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: [<ffffffff8126e051>] keyring_destroy+0x3d/0x88
	...
	Workqueue: events key_garbage_collector
	...
	RIP: 0010:[<ffffffff8126e051>] 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:
	 [<ffffffff8126c756>] key_gc_unused_keys.constprop.1+0x5d/0x10f
	 [<ffffffff8126ca71>] key_garbage_collector+0x1fa/0x351
	 [<ffffffff8105ec9b>] process_one_work+0x28e/0x547
	 [<ffffffff8105fd17>] worker_thread+0x26e/0x361
	 [<ffffffff8105faa9>] ? rescuer_thread+0x2a8/0x2a8
	 [<ffffffff810648ad>] kthread+0xf3/0xfb
	 [<ffffffff810647ba>] ? kthread_create_on_node+0x1c2/0x1c2
	 [<ffffffff815f2ccf>] ret_from_fork+0x3f/0x70
	 [<ffffffff810647ba>] ? 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: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Tested-by: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      aad1f1b8
    • David Howells's avatar
      KEYS: Fix race between key destruction and finding a keyring by name · b49c4dd1
      David Howells authored 
      commit 94c4554b 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: default avatarPetr Matousek <pmatouse@redhat.com>
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      b49c4dd1
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7