| From 38327424b40bcebe2de92d07312c89360ac9229a Mon Sep 17 00:00:00 2001 |
| From: Dan Carpenter <dan.carpenter@oracle.com> |
| Date: Thu, 16 Jun 2016 15:48:57 +0100 |
| Subject: KEYS: potential uninitialized variable |
| |
| From: Dan Carpenter <dan.carpenter@oracle.com> |
| |
| 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:[<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: |
| 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 <dan.carpenter@oracle.com> |
| Signed-off-by: David Howells <dhowells@redhat.com> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| security/keys/key.c | 2 +- |
| 1 file changed, 1 insertion(+), 1 deletion(-) |
| |
| --- a/security/keys/key.c |
| +++ b/security/keys/key.c |
| @@ -578,7 +578,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 */ |