diff options
author | Jann Horn <jannh@google.com> | 2019-03-27 11:39:38 -0400 |
---|---|---|
committer | James Morris <james.morris@microsoft.com> | 2019-04-10 13:28:21 -0400 |
commit | 5c7e372caa35d303e414caeb64ee2243fd3cac3d (patch) | |
tree | 0ebe3b3d6fa9becd9cfa6e5a49f42c7979be8180 | |
parent | 1b26fcdb748eb20a73f72900d7f5ab537b2809be (diff) |
security: don't use RCU accessors for cred->session_keyring
sparse complains that a bunch of places in kernel/cred.c access
cred->session_keyring without the RCU helpers required by the __rcu
annotation.
cred->session_keyring is written in the following places:
- prepare_kernel_cred() [in a new cred struct]
- keyctl_session_to_parent() [in a new cred struct]
- prepare_creds [in a new cred struct, via memcpy]
- install_session_keyring_to_cred()
- from install_session_keyring() on new creds
- from join_session_keyring() on new creds [twice]
- from umh_keys_init()
- from call_usermodehelper_exec_async() on new creds
All of these writes are before the creds are committed; therefore,
cred->session_keyring doesn't need RCU protection.
Remove the __rcu annotation and fix up all existing users that use __rcu.
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: James Morris <james.morris@microsoft.com>
-rw-r--r-- | include/linux/cred.h | 2 | ||||
-rw-r--r-- | security/keys/process_keys.c | 12 | ||||
-rw-r--r-- | security/keys/request_key.c | 9 |
3 files changed, 7 insertions, 16 deletions
diff --git a/include/linux/cred.h b/include/linux/cred.h index ddd45bb74887..efb6edf32de7 100644 --- a/include/linux/cred.h +++ b/include/linux/cred.h | |||
@@ -138,7 +138,7 @@ struct cred { | |||
138 | #ifdef CONFIG_KEYS | 138 | #ifdef CONFIG_KEYS |
139 | unsigned char jit_keyring; /* default keyring to attach requested | 139 | unsigned char jit_keyring; /* default keyring to attach requested |
140 | * keys to */ | 140 | * keys to */ |
141 | struct key __rcu *session_keyring; /* keyring inherited over fork */ | 141 | struct key *session_keyring; /* keyring inherited over fork */ |
142 | struct key *process_keyring; /* keyring private to this process */ | 142 | struct key *process_keyring; /* keyring private to this process */ |
143 | struct key *thread_keyring; /* keyring private to this thread */ | 143 | struct key *thread_keyring; /* keyring private to this thread */ |
144 | struct key *request_key_auth; /* assumed request_key authority */ | 144 | struct key *request_key_auth; /* assumed request_key authority */ |
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index 9320424c4a46..bd7243cb4c85 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c | |||
@@ -227,6 +227,7 @@ static int install_process_keyring(void) | |||
227 | * Install the given keyring as the session keyring of the given credentials | 227 | * Install the given keyring as the session keyring of the given credentials |
228 | * struct, replacing the existing one if any. If the given keyring is NULL, | 228 | * struct, replacing the existing one if any. If the given keyring is NULL, |
229 | * then install a new anonymous session keyring. | 229 | * then install a new anonymous session keyring. |
230 | * @cred can not be in use by any task yet. | ||
230 | * | 231 | * |
231 | * Return: 0 on success; -errno on failure. | 232 | * Return: 0 on success; -errno on failure. |
232 | */ | 233 | */ |
@@ -254,7 +255,7 @@ int install_session_keyring_to_cred(struct cred *cred, struct key *keyring) | |||
254 | 255 | ||
255 | /* install the keyring */ | 256 | /* install the keyring */ |
256 | old = cred->session_keyring; | 257 | old = cred->session_keyring; |
257 | rcu_assign_pointer(cred->session_keyring, keyring); | 258 | cred->session_keyring = keyring; |
258 | 259 | ||
259 | if (old) | 260 | if (old) |
260 | key_put(old); | 261 | key_put(old); |
@@ -392,11 +393,8 @@ key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) | |||
392 | 393 | ||
393 | /* search the session keyring */ | 394 | /* search the session keyring */ |
394 | if (ctx->cred->session_keyring) { | 395 | if (ctx->cred->session_keyring) { |
395 | rcu_read_lock(); | ||
396 | key_ref = keyring_search_aux( | 396 | key_ref = keyring_search_aux( |
397 | make_key_ref(rcu_dereference(ctx->cred->session_keyring), 1), | 397 | make_key_ref(ctx->cred->session_keyring, 1), ctx); |
398 | ctx); | ||
399 | rcu_read_unlock(); | ||
400 | 398 | ||
401 | if (!IS_ERR(key_ref)) | 399 | if (!IS_ERR(key_ref)) |
402 | goto found; | 400 | goto found; |
@@ -612,10 +610,8 @@ try_again: | |||
612 | goto reget_creds; | 610 | goto reget_creds; |
613 | } | 611 | } |
614 | 612 | ||
615 | rcu_read_lock(); | 613 | key = ctx.cred->session_keyring; |
616 | key = rcu_dereference(ctx.cred->session_keyring); | ||
617 | __key_get(key); | 614 | __key_get(key); |
618 | rcu_read_unlock(); | ||
619 | key_ref = make_key_ref(key, 1); | 615 | key_ref = make_key_ref(key, 1); |
620 | break; | 616 | break; |
621 | 617 | ||
diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 2f17d84d46f1..db72dc4d7639 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c | |||
@@ -142,12 +142,10 @@ static int call_sbin_request_key(struct key *authkey, void *aux) | |||
142 | prkey = cred->process_keyring->serial; | 142 | prkey = cred->process_keyring->serial; |
143 | sprintf(keyring_str[1], "%d", prkey); | 143 | sprintf(keyring_str[1], "%d", prkey); |
144 | 144 | ||
145 | rcu_read_lock(); | 145 | session = cred->session_keyring; |
146 | session = rcu_dereference(cred->session_keyring); | ||
147 | if (!session) | 146 | if (!session) |
148 | session = cred->user->session_keyring; | 147 | session = cred->user->session_keyring; |
149 | sskey = session->serial; | 148 | sskey = session->serial; |
150 | rcu_read_unlock(); | ||
151 | 149 | ||
152 | sprintf(keyring_str[2], "%d", sskey); | 150 | sprintf(keyring_str[2], "%d", sskey); |
153 | 151 | ||
@@ -287,10 +285,7 @@ static int construct_get_dest_keyring(struct key **_dest_keyring) | |||
287 | 285 | ||
288 | /* fall through */ | 286 | /* fall through */ |
289 | case KEY_REQKEY_DEFL_SESSION_KEYRING: | 287 | case KEY_REQKEY_DEFL_SESSION_KEYRING: |
290 | rcu_read_lock(); | 288 | dest_keyring = key_get(cred->session_keyring); |
291 | dest_keyring = key_get( | ||
292 | rcu_dereference(cred->session_keyring)); | ||
293 | rcu_read_unlock(); | ||
294 | 289 | ||
295 | if (dest_keyring) | 290 | if (dest_keyring) |
296 | break; | 291 | break; |