linux-stable.git/security/keys, branch v3.16.35

KEYS: Fix keyring ref leak in join_session_keyring()

2016-01-25T10:44:08+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: Luis Henriques

KEYS: Fix race between read and revoke

2016-01-05T13:18:04+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 
Cc: Moritz Muehlenhoff 
Signed-off-by: Luis Henriques

KEYS: Don't permit request_key() to construct a new keyring

2015-10-28T10:33:23+00:00

commit 911b79cde95c7da0ec02f48105358a36636b7a71 upstream.

If request_key() is used to find a keyring, only do the search part - don't
do the construction part if the keyring was not found by the search.  We
don't really want keyrings in the negative instantiated state since the
rejected/negative instantiation error value in the payload is unioned with
keyring metadata.

Now the kernel gives an error:

	request_key("keyring", "#selinux,bdekeyring", "keyring", KEY_SPEC_USER_SESSION_KEYRING) = -1 EPERM (Operation not permitted)

Signed-off-by: David Howells 
Cc: Kamal Mostafa 
Signed-off-by: Luis Henriques

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

2015-10-28T10:33:23+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 
Cc: Kamal Mostafa 
Signed-off-by: Luis Henriques

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

2015-10-28T10:33:22+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 
Cc: Kamal Mostafa 
Signed-off-by: Luis Henriques

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

2015-08-11T08:57:27+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 
Cc: Moritz Mühlenhoff 
Signed-off-by: Luis Henriques

KEYS: close race between key lookup and freeing

2015-01-15T10:44:59+00:00

commit a3a8784454692dd72e5d5d34dcdab17b4420e74c upstream.

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 
Cc: Moritz Muehlenhoff 
Signed-off-by: Luis Henriques

KEYS: Fix stale key registration at error path

2015-01-15T10:43:37+00:00

commit b26bdde5bb27f3f900e25a95e33a0c476c8c2c48 upstream.

When loading encrypted-keys module, if the last check of
aes_get_sizes() in init_encrypted() fails, the driver just returns an
error without unregistering its key type.  This results in the stale
entry in the list.  In addition to memory leaks, this leads to a kernel
crash when registering a new key type later.

This patch fixes the problem by swapping the calls of aes_get_sizes()
and register_key_type(), and releasing resources properly at the error
paths.

Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=908163
Signed-off-by: Takashi Iwai 
Signed-off-by: Mimi Zohar 
Signed-off-by: Luis Henriques

Merge branch 'serge-next-1' of git://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux-security

2014-06-10T17:05:36+00:00

Pull security layer updates from Serge Hallyn:
 "This is a merge of James Morris' security-next tree from 3.14 to
  yesterday's master, plus four patches from Paul Moore which are in
  linux-next, plus one patch from Mimi"

* 'serge-next-1' of git://git.kernel.org/pub/scm/linux/kernel/git/sergeh/linux-security:
  ima: audit log files opened with O_DIRECT flag
  selinux: conditionally reschedule in hashtab_insert while loading selinux policy
  selinux: conditionally reschedule in mls_convert_context while loading selinux policy
  selinux: reject setexeccon() on MNT_NOSUID applications with -EACCES
  selinux:  Report permissive mode in avc: denied messages.
  Warning in scanf string typing
  Smack: Label cgroup files for systemd
  Smack: Verify read access on file open - v3
  security: Convert use of typedef ctl_table to struct ctl_table
  Smack: bidirectional UDS connect check
  Smack: Correctly remove SMACK64TRANSMUTE attribute
  SMACK: Fix handling value==NULL in post setxattr
  bugfix patch for SMACK
  Smack: adds smackfs/ptrace interface
  Smack: unify all ptrace accesses in the smack
  Smack: fix the subject/object order in smack_ptrace_traceme()
  Minor improvement of 'smack_sb_kern_mount'
  smack: fix key permission verification
  KEYS: Move the flags representing required permission to linux/key.h

security: Convert use of typedef ctl_table to struct ctl_table

2014-04-15T03:39:58+00:00

This typedef is unnecessary and should just be removed.

Signed-off-by: Joe Perches 
Signed-off-by: James Morris