[PATCH] KEYS: potential uninitialized variable

From: David Howells
Date: Thu Jun 16 2016 - 10:49:10 EST


From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>

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:[<ffffffff811600f9>] 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:
[<ffffffff8116429f>] kfree+0xde/0x1bc
[<ffffffff81318a66>] assoc_array_cancel_edit+0x1f/0x36
[<ffffffff812922a2>] __key_link_end+0x55/0x63
[<ffffffff81290519>] key_reject_and_link+0x124/0x155
[<ffffffff81293728>] keyctl_reject_key+0xb6/0xe0
[<ffffffff81293762>] keyctl_negate_key+0x10/0x12
[<ffffffff81293dff>] SyS_keyctl+0x9f/0xe7
[<ffffffff81001ce1>] do_syscall_64+0x63/0x13a
[<ffffffff81601d1a>] entry_SYSCALL64_slow_path+0x25/0x25

Fixes: f70e2e06196a ('KEYS: Do preallocation for __key_link()')
Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
cc: stable@xxxxxxxxxxxxxxx
---

security/keys/key.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/key.c b/security/keys/key.c
index bd5a272f28a6..346fbf201c22 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -597,7 +597,7 @@ int key_reject_and_link(struct key *key,

mutex_unlock(&key_construction_mutex);

- if (keyring)
+ if (keyring && link_ret == 0)
__key_link_end(keyring, &key->index_key, edit);

/* wake up anyone waiting for a key to be constructed */