Re: [PATCH] security/keys: fix slab-out-of-bounds in key_task_permission

From: Chen Ridong
Date: Wed Sep 18 2024 - 03:47:41 EST




On 2024/9/15 21:59, Jarkko Sakkinen wrote:
On Sun Sep 15, 2024 at 3:55 AM EEST, Chen Ridong wrote:


On 2024/9/14 19:33, Jarkko Sakkinen wrote:
On Fri Sep 13, 2024 at 10:09 AM EEST, Chen Ridong wrote:
We meet the same issue with the LINK, which reads memory out of bounds:

Nit: don't use "we" anywhere".

Tbh, I really don't understand the sentence above. I don't what
"the same issue with the LINK" really is.


Hello, Jarkko.
I apologize for any confusion caused.

I've encountered a bug reported by syzkaller. I also found the same bug
reported at this LINK:
https://syzkaller.appspot.com/bug?id=68a5e206c2a8e08d317eb83f05610c0484ad10b9.

BUG: KASAN: slab-out-of-bounds in __kuid_val include/linux/uidgid.h:36
BUG: KASAN: slab-out-of-bounds in uid_eq include/linux/uidgid.h:63 [inline]
BUG: KASAN: slab-out-of-bounds in key_task_permission+0x394/0x410
security/keys/permission.c:54
Read of size 4 at addr ffff88813c3ab618 by task stress-ng/4362

CPU: 2 PID: 4362 Comm: stress-ng Not tainted 5.10.0-14930-gafbffd6c3ede #15
Call Trace:
__dump_stack lib/dump_stack.c:82 [inline]
dump_stack+0x107/0x167 lib/dump_stack.c:123
print_address_description.constprop.0+0x19/0x170 mm/kasan/report.c:400
__kasan_report.cold+0x6c/0x84 mm/kasan/report.c:560
kasan_report+0x3a/0x50 mm/kasan/report.c:585
__kuid_val include/linux/uidgid.h:36 [inline]
uid_eq include/linux/uidgid.h:63 [inline]
key_task_permission+0x394/0x410 security/keys/permission.c:54
search_nested_keyrings+0x90e/0xe90 security/keys/keyring.c:793
keyring_search_rcu+0x1b6/0x310 security/keys/keyring.c:922
search_cred_keyrings_rcu+0x111/0x2e0 security/keys/process_keys.c:459
search_process_keyrings_rcu+0x1d/0x310 security/keys/process_keys.c:544
lookup_user_key+0x782/0x12e0 security/keys/process_keys.c:762
keyctl_invalidate_key+0x20/0x190 security/keys/keyctl.c:434
__do_sys_keyctl security/keys/keyctl.c:1978 [inline]
__se_sys_keyctl+0x1de/0x5b0 security/keys/keyctl.c:1880
do_syscall_64+0x30/0x40 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x67/0xd1

However, we can't reproduce this issue.

"The issue cannot be easily reproduced but by analyzing the code
it can be broken into following steps:"

Thank you for your correction.
Does this patch address the issue correctly? Is this patch acceptable?

I only comment new patch versions so not giving any promises but I can
say that it is I think definitely in the correct direction :-)

BR, Jarkko

Hello, Jarkko. I have reproduced this issue. It can be reproduced by following these steps:

1. Add the helper patch.

@@ -205,6 +205,9 @@ static void hash_key_type_and_desc(struct keyring_index_key *index_key)
else if (index_key->type == &key_type_keyring && (hash & fan_mask) != 0)
hash = (hash + (hash << level_shift)) & ~fan_mask;
index_key->hash = hash;
+ if ((index_key->hash & 0xff) == 0xe6) {
+ pr_err("hash_key_type_and_desc: type %s %s 0x%x\n", index_key->type->name, index_key->description, index_key->hash);
+ }
}

2. Pick up the inputs whose hash is xxe6 using the following cmd. If a key's hash is xxe6, it will be printed.

for ((i=0; i<=10000; i++)); do ./test_key user user$i "Some payload"; done

You have complile test_key whith following code.

#include <sys/types.h>
#include <keyutils.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int
main(int argc, char *argv[])
{
key_serial_t key;

if (argc != 4) {
fprintf(stderr, "Usage: %s type description payload\n",
argv[0]);
exit(EXIT_FAILURE);
}

key = add_key(argv[1], argv[2], argv[3], strlen(argv[3]),
KEY_SPEC_SESSION_KEYRING);
if (key == -1) {
perror("add_key");
exit(EXIT_FAILURE);
}

printf("Key ID is %jx\n", (uintmax_t) key);

exit(EXIT_SUCCESS);
}


3. Have more than 32 inputs now. their hashes are xxe6.
eg.
hash_key_type_and_desc: type user user438 0xe3033fe6
hash_key_type_and_desc: type user user526 0xeb7eade6
...
hash_key_type_and_desc: type user user9955 0x44bc99e6

4. Reboot and add the keys obtained from step 3.
When adding keys to the ROOT that their hashes are all xxe6, and up to 16, the ROOT has keys with hashes that are not xxe6 (e.g., slot 0), so the keys are dissimilar. The ROOT will then split NODE A without using a shortcut. When NODE A is filled with keys that have hashes of xxe6, the keys are similar. NODE A will split with a shortcut.

As my analysis, if a slot of the root is a shortcut(slot 6), it may be mistakenly be transferred to a key*, leading to an read out-of-bounds read.

NODE A
+------>+---+
ROOT | | 0 | xxe6
+---+ | +---+
xxxx | 0 | shortcut : : xxe6
+---+ | +---+
xxe6 : : | | | xxe6
+---+ | +---+
| 6 |---+ : : xxe6
+---+ +---+
xxe6 : : | f | xxe6
+---+ +---+
xxe6 | f |
+---+

5. cat /proc/keys. and the issue is reproduced.