diff options
author | David Howells <dhowells@redhat.com> | 2014-03-09 08:21:58 +0000 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2014-03-09 18:57:18 -0700 |
commit | 979e0d74651ba5aa533277f2a6423d0f982fb6f6 (patch) | |
tree | 69db73a995fe850fc83dd14fd3acd52653692407 /security | |
parent | 1dc3217dad3e771f2bd12703d6daed6cb818fdd8 (diff) | |
download | linux-979e0d74651ba5aa533277f2a6423d0f982fb6f6.tar.gz linux-979e0d74651ba5aa533277f2a6423d0f982fb6f6.tar.bz2 linux-979e0d74651ba5aa533277f2a6423d0f982fb6f6.zip |
KEYS: Make the keyring cycle detector ignore other keyrings of the same 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 <tt.rantala@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: James Morris <james.l.morris@oracle.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'security')
-rw-r--r-- | security/keys/keyring.c | 6 |
1 files changed, 5 insertions, 1 deletions
diff --git a/security/keys/keyring.c b/security/keys/keyring.c index d46cbc5e335e..2fb2576dc644 100644 --- a/security/keys/keyring.c +++ b/security/keys/keyring.c @@ -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; } |