]>
Commit | Line | Data |
---|---|---|
502b719a GKH |
1 | From ede0fa98a900e657d1fcd80b50920efc896c1a4c Mon Sep 17 00:00:00 2001 |
2 | From: Eric Biggers <ebiggers@google.com> | |
3 | Date: Fri, 22 Feb 2019 15:36:18 +0000 | |
4 | Subject: KEYS: always initialize keyring_index_key::desc_len | |
5 | ||
6 | From: Eric Biggers <ebiggers@google.com> | |
7 | ||
8 | commit ede0fa98a900e657d1fcd80b50920efc896c1a4c upstream. | |
9 | ||
10 | syzbot hit the 'BUG_ON(index_key->desc_len == 0);' in __key_link_begin() | |
11 | called from construct_alloc_key() during sys_request_key(), because the | |
12 | length of the key description was never calculated. | |
13 | ||
14 | The problem is that we rely on ->desc_len being initialized by | |
15 | search_process_keyrings(), specifically by search_nested_keyrings(). | |
16 | But, if the process isn't subscribed to any keyrings that never happens. | |
17 | ||
18 | Fix it by always initializing keyring_index_key::desc_len as soon as the | |
19 | description is set, like we already do in some places. | |
20 | ||
21 | The following program reproduces the BUG_ON() when it's run as root and | |
22 | no session keyring has been installed. If it doesn't work, try removing | |
23 | pam_keyinit.so from /etc/pam.d/login and rebooting. | |
24 | ||
25 | #include <stdlib.h> | |
26 | #include <unistd.h> | |
27 | #include <keyutils.h> | |
28 | ||
29 | int main(void) | |
30 | { | |
31 | int id = add_key("keyring", "syz", NULL, 0, KEY_SPEC_USER_KEYRING); | |
32 | ||
33 | keyctl_setperm(id, KEY_OTH_WRITE); | |
34 | setreuid(5000, 5000); | |
35 | request_key("user", "desc", "", id); | |
36 | } | |
37 | ||
38 | Reported-by: syzbot+ec24e95ea483de0a24da@syzkaller.appspotmail.com | |
39 | Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring") | |
40 | Signed-off-by: Eric Biggers <ebiggers@google.com> | |
41 | Signed-off-by: David Howells <dhowells@redhat.com> | |
42 | Cc: stable@vger.kernel.org | |
43 | Signed-off-by: James Morris <james.morris@microsoft.com> | |
44 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
45 | ||
46 | --- | |
47 | security/keys/keyring.c | 4 +--- | |
48 | security/keys/proc.c | 3 +-- | |
49 | security/keys/request_key.c | 1 + | |
50 | security/keys/request_key_auth.c | 2 +- | |
51 | 4 files changed, 4 insertions(+), 6 deletions(-) | |
52 | ||
53 | --- a/security/keys/keyring.c | |
54 | +++ b/security/keys/keyring.c | |
55 | @@ -628,9 +628,6 @@ static bool search_nested_keyrings(struc | |
56 | BUG_ON((ctx->flags & STATE_CHECKS) == 0 || | |
57 | (ctx->flags & STATE_CHECKS) == STATE_CHECKS); | |
58 | ||
59 | - if (ctx->index_key.description) | |
60 | - ctx->index_key.desc_len = strlen(ctx->index_key.description); | |
61 | - | |
62 | /* Check to see if this top-level keyring is what we are looking for | |
63 | * and whether it is valid or not. | |
64 | */ | |
65 | @@ -888,6 +885,7 @@ key_ref_t keyring_search(key_ref_t keyri | |
66 | struct keyring_search_context ctx = { | |
67 | .index_key.type = type, | |
68 | .index_key.description = description, | |
69 | + .index_key.desc_len = strlen(description), | |
70 | .cred = current_cred(), | |
71 | .match_data.cmp = key_default_cmp, | |
72 | .match_data.raw_data = description, | |
73 | --- a/security/keys/proc.c | |
74 | +++ b/security/keys/proc.c | |
75 | @@ -186,8 +186,7 @@ static int proc_keys_show(struct seq_fil | |
76 | int rc; | |
77 | ||
78 | struct keyring_search_context ctx = { | |
79 | - .index_key.type = key->type, | |
80 | - .index_key.description = key->description, | |
81 | + .index_key = key->index_key, | |
82 | .cred = current_cred(), | |
83 | .match_data.cmp = lookup_user_key_possessed, | |
84 | .match_data.raw_data = key, | |
85 | --- a/security/keys/request_key.c | |
86 | +++ b/security/keys/request_key.c | |
87 | @@ -544,6 +544,7 @@ struct key *request_key_and_link(struct | |
88 | struct keyring_search_context ctx = { | |
89 | .index_key.type = type, | |
90 | .index_key.description = description, | |
91 | + .index_key.desc_len = strlen(description), | |
92 | .cred = current_cred(), | |
93 | .match_data.cmp = key_default_cmp, | |
94 | .match_data.raw_data = description, | |
95 | --- a/security/keys/request_key_auth.c | |
96 | +++ b/security/keys/request_key_auth.c | |
97 | @@ -254,7 +254,7 @@ struct key *key_get_instantiation_authke | |
98 | struct key *authkey; | |
99 | key_ref_t authkey_ref; | |
100 | ||
101 | - sprintf(description, "%x", target_id); | |
102 | + ctx.index_key.desc_len = sprintf(description, "%x", target_id); | |
103 | ||
104 | authkey_ref = search_process_keyrings(&ctx); | |
105 |