linux-stable.git/security/keys/key.c, branch v3.2.85

KEYS: potential uninitialized variable

2016-08-22T21:37:14+00:00

commit 38327424b40bcebe2de92d07312c89360ac9229a upstream.

If __key_link_begin() failed then "edit" would be uninitialized.  I've
added a check to fix that.

This allows a random user to crash the kernel, though it's quite
difficult to achieve.  There are three ways it can be done as the user
would have to cause an error to occur in __key_link():

 (1) Cause the kernel to run out of memory.  In practice, this is difficult
     to achieve without ENOMEM cropping up elsewhere and aborting the
     attempt.

 (2) Revoke the destination keyring between the keyring ID being looked up
     and it being tested for revocation.  In practice, this is difficult to
     time correctly because the KEYCTL_REJECT function can only be used
     from the request-key upcall process.  Further, users can only make use
     of what's in /sbin/request-key.conf, though this does including a
     rejection debugging test - which means that the destination keyring
     has to be the caller's session keyring in practice.

 (3) Have just enough key quota available to create a key, a new session
     keyring for the upcall and a link in the session keyring, but not then
     sufficient quota to create a link in the nominated destination keyring
     so that it fails with EDQUOT.

The bug can be triggered using option (3) above using something like the
following:

	echo 80 >/proc/sys/kernel/keys/root_maxbytes
	keyctl request2 user debug:fred negate @t

The above sets the quota to something much lower (80) to make the bug
easier to trigger, but this is dependent on the system.  Note also that
the name of the keyring created contains a random number that may be
between 1 and 10 characters in size, so may throw the test off by
changing the amount of quota used.

Assuming the failure occurs, something like the following will be seen:

	kfree_debugcheck: out of range ptr 6b6b6b6b6b6b6b68h
	------------[ cut here ]------------
	kernel BUG at ../mm/slab.c:2821!
	...
	RIP: 0010:[] kfree_debugcheck+0x20/0x25
	RSP: 0018:ffff8804014a7de8  EFLAGS: 00010092
	RAX: 0000000000000034 RBX: 6b6b6b6b6b6b6b68 RCX: 0000000000000000
	RDX: 0000000000040001 RSI: 00000000000000f6 RDI: 0000000000000300
	RBP: ffff8804014a7df0 R08: 0000000000000001 R09: 0000000000000000
	R10: ffff8804014a7e68 R11: 0000000000000054 R12: 0000000000000202
	R13: ffffffff81318a66 R14: 0000000000000000 R15: 0000000000000001
	...
	Call Trace:
	  kfree+0xde/0x1bc
	  assoc_array_cancel_edit+0x1f/0x36
	  __key_link_end+0x55/0x63
	  key_reject_and_link+0x124/0x155
	  keyctl_reject_key+0xb6/0xe0
	  keyctl_negate_key+0x10/0x12
	  SyS_keyctl+0x9f/0xe7
	  do_syscall_64+0x63/0x13a
	  entry_SYSCALL64_slow_path+0x25/0x25

Fixes: f70e2e06196a ('KEYS: Do preallocation for __key_link()')
Signed-off-by: Dan Carpenter 
Signed-off-by: David Howells 
Signed-off-by: Linus Torvalds 
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings

KEYS: Correctly destroy key payloads when their keytype is removed

2011-08-22T23:57:37+00:00

unregister_key_type() has code to mark a key as dead and make it unavailable in
one loop and then destroy all those unavailable key payloads in the next loop.
However, the loop to mark keys dead renders the key undetectable to the second
loop by changing the key type pointer also.

Fix this by the following means:

 (1) The key code has two garbage collectors: one deletes unreferenced keys and
     the other alters keyrings to delete links to old dead, revoked and expired
     keys.  They can end up holding each other up as both want to scan the key
     serial tree under spinlock.  Combine these into a single routine.

 (2) Move the dead key marking, dead link removal and dead key removal into the
     garbage collector as a three phase process running over the three cycles
     of the normal garbage collection procedure.  This is tracked by the
     KEY_GC_REAPING_DEAD_1, _2 and _3 state flags.

     unregister_key_type() then just unlinks the key type from the list, wakes
     up the garbage collector and waits for the third phase to complete.

 (3) Downgrade the key types sem in unregister_key_type() once it has deleted
     the key type from the list so that it doesn't block the keyctl() syscall.

 (4) Dead keys that cannot be simply removed in the third phase have their
     payloads destroyed with the key's semaphore write-locked to prevent
     interference by the keyctl() syscall.  There should be no in-kernel users
     of dead keys of that type by the point of unregistration, though keyctl()
     may be holding a reference.

 (5) Only perform timer recalculation in the GC if the timer actually expired.
     If it didn't, we'll get another cycle when it goes off - and if the key
     that actually triggered it has been removed, it's not a problem.

 (6) Only garbage collect link if the timer expired or if we're doing dead key
     clean up phase 2.

 (7) As only key_garbage_collector() is permitted to use rb_erase() on the key
     serial tree, it doesn't need to revalidate its cursor after dropping the
     spinlock as the node the cursor points to must still exist in the tree.

 (8) Drop the spinlock in the GC if there is contention on it or if we need to
     reschedule.  After dealing with that, get the spinlock again and resume
     scanning.

This has been tested in the following ways:

 (1) Run the keyutils testsuite against it.

 (2) Using the AF_RXRPC and RxKAD modules to test keytype removal:

     Load the rxrpc_s key type:

	# insmod /tmp/af-rxrpc.ko
	# insmod /tmp/rxkad.ko

     Create a key (http://people.redhat.com/~dhowells/rxrpc/listen.c):

	# /tmp/listen &
	[1] 8173

     Find the key:

	# grep rxrpc_s /proc/keys
	091086e1 I--Q--     1 perm 39390000     0     0 rxrpc_s   52:2

     Link it to a session keyring, preferably one with a higher serial number:

	# keyctl link 0x20e36251 @s

     Kill the process (the key should remain as it's linked to another place):

	# fg
	/tmp/listen
	^C

     Remove the key type:

	rmmod rxkad
	rmmod af-rxrpc

     This can be made a more effective test by altering the following part of
     the patch:

	if (unlikely(gc_state & KEY_GC_REAPING_DEAD_2)) {
		/* Make sure everyone revalidates their keys if we marked a
		 * bunch as being dead and make sure all keyring ex-payloads
		 * are destroyed.
		 */
		kdebug("dead sync");
		synchronize_rcu();

     To call synchronize_rcu() in GC phase 1 instead.  That causes that the
     keyring's old payload content to hang around longer until it's RCU
     destroyed - which usually happens after GC phase 3 is complete.  This
     allows the destroy_dead_key branch to be tested.

Reported-by: Benjamin Coddington 
Signed-off-by: David Howells 
Signed-off-by: James Morris

KEYS: Make the key reaper non-reentrant

2011-08-22T23:57:36+00:00

Make the key reaper non-reentrant by sticking it on the appropriate system work
queue when we queue it.  This will allow it to have global state and drop
locks.  It should probably be non-reentrant already as it may spend a long time
holding the key serial spinlock, and so multiple entrants can spend long
periods of time just sitting there spinning, waiting to get the lock.

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

KEYS: Move the unreferenced key reaper to the keys garbage collector file

2011-08-22T23:57:36+00:00

Move the unreferenced key reaper function to the keys garbage collector file
as that's a more appropriate place with the dead key link reaper.

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

KEYS: Add a new keyctl op to reject a key with a specified error code

2011-03-08T00:17:18+00:00

Add a new keyctl op to reject a key with a specified error code.  This works
much the same as negating a key, and so keyctl_negate_key() is made a special
case of keyctl_reject_key().  The difference is that keyctl_negate_key()
selects ENOKEY as the error to be reported.

Typically the key would be rejected with EKEYEXPIRED, EKEYREVOKED or
EKEYREJECTED, but this is not mandatory.

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

KEYS: Add a key type op to permit the key description to be vetted

2011-03-08T00:17:15+00:00

Add a key type operation to permit the key type to vet the description of a new
key that key_alloc() is about to allocate.  The operation may reject the
description if it wishes with an error of its choosing.  If it does this, the
key will not be allocated.

Signed-off-by: David Howells 
Reviewed-by: Mimi Zohar 
Signed-off-by: James Morris

KEYS: Fix __key_link_end() quota fixup on error

2011-01-25T22:58:20+00:00

Fix __key_link_end()'s attempt to fix up the quota if an error occurs.

There are two erroneous cases: Firstly, we always decrease the quota if
the preallocated replacement keyring needs cleaning up, irrespective of
whether or not we should (we may have replaced a pointer rather than
adding another pointer).

Secondly, we never clean up the quota if we added a pointer without the
keyring storage being extended (we allocate multiple pointers at a time,
even if we're not going to use them all immediately).

We handle this by setting the bottom bit of the preallocation pointer in
__key_link_begin() to indicate that the quota needs fixing up, which is
then passed to __key_link() (which clears the whole thing) and
__key_link_end().

Signed-off-by: David Howells 
Signed-off-by: Linus Torvalds

KEYS: Fix up comments in key management code

2011-01-21T22:59:30+00:00

Fix up comments in the key management code.  No functional changes.

Signed-off-by: David Howells 
Signed-off-by: Linus Torvalds

KEYS: Do some style cleanup in the key management code.

2011-01-21T22:59:29+00:00

Do a bit of a style clean up in the key management code.  No functional
changes.

Done using:

  perl -p -i -e 's!^/[*]*/\n!!' security/keys/*.c
  perl -p -i -e 's!} /[*] end [a-z0-9_]*[(][)] [*]/\n!}\n!' security/keys/*.c
  sed -i -s -e ": next" -e N -e 's/^\n[}]$/}/' -e t -e P -e 's/^.*\n//' -e "b next" security/keys/*.c

To remove /*****/ lines, remove comments on the closing brace of a
function to name the function and remove blank lines before the closing
brace of a function.

Signed-off-by: David Howells 
Signed-off-by: Linus Torvalds

KEYS: Do preallocation for __key_link()

2010-05-06T12:25:02+00:00

Do preallocation for __key_link() so that the various callers in request_key.c
can deal with any errors from this source before attempting to construct a key.
This allows them to assume that the actual linkage step is guaranteed to be
successful.

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