Skip to content

Commit

Permalink
KEYS: Make the keyring cycle detector ignore other keyrings of the sa…
Browse files Browse the repository at this point in the history
…me name

This fixes CVE-2014-0102.

The following command sequence produces an oops:

	keyctl new_session
	i=`keyctl newring _ses @s`
	keyctl link @s $i

The problem is that search_nested_keyrings() sees two keyrings that have
matching type and description, so keyring_compare_object() returns true.
s_n_k() then passes the key to the iterator function -
keyring_detect_cycle_iterator() - which *should* check to see whether this is
the keyring of interest, not just one with the same name.

Because assoc_array_find() will return one and only one match, I assumed that
the iterator function would only see an exact match or never be called - but
the iterator isn't only called from assoc_array_find()...

The oops looks something like this:

	kernel BUG at /data/fs/linux-2.6-fscache/security/keys/keyring.c:1003!
	invalid opcode: 0000 [#1] SMP
	...
	RIP: keyring_detect_cycle_iterator+0xe/0x1f
	...
	Call Trace:
	  search_nested_keyrings+0x76/0x2aa
	  __key_link_check_live_key+0x50/0x5f
	  key_link+0x4e/0x85
	  keyctl_keyring_link+0x60/0x81
	  SyS_keyctl+0x65/0xe4
	  tracesys+0xdd/0xe2

The fix is to make keyring_detect_cycle_iterator() check that the key it
has is the key it was actually looking for rather than calling BUG_ON().

A testcase has been included in the keyutils testsuite for this:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/keyutils.git/commit/?id=891f3365d07f1996778ade0e3428f01878a1790b

Reported-by: Tommi Rantala <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: James Morris <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
dhowells authored and torvalds committed Mar 10, 2014
1 parent 1dc3217 commit 979e0d7
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion security/keys/keyring.c
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,11 @@ static int keyring_detect_cycle_iterator(const void *object,

kenter("{%d}", key->serial);

BUG_ON(key != ctx->match_data);
/* We might get a keyring with matching index-key that is nonetheless a
* different keyring. */
if (key != ctx->match_data)
return 0;

ctx->result = ERR_PTR(-EDEADLK);
return 1;
}
Expand Down

0 comments on commit 979e0d7

Please sign in to comment.