aboutsummaryrefslogtreecommitdiffstats
path: root/security/keys
diff options
context:
space:
mode:
authorDavid Howells <dhowells@redhat.com>2006-06-22 17:47:18 -0400
committerLinus Torvalds <torvalds@g5.osdl.org>2006-06-22 18:05:56 -0400
commit04c567d9313e4927b9835361d8ac0318ce65af6b (patch)
treed040ef59337342603f2cc30917493fb6a74a212a /security/keys
parentd720024e94de4e8b7f10ee83c532926f3ad5d708 (diff)
[PATCH] Keys: Fix race between two instantiators of a key
Add a revocation notification method to the key type and calls it whilst the key's semaphore is still write-locked after setting the revocation flag. The patch then uses this to maintain a reference on the task_struct of the process that calls request_key() for as long as the authorisation key remains unrevoked. This fixes a potential race between two processes both of which have assumed the authority to instantiate a key (one may have forked the other for example). The problem is that there's no locking around the check for revocation of the auth key and the use of the task_struct it points to, nor does the auth key keep a reference on the task_struct. Access to the "context" pointer in the auth key must thenceforth be done with the auth key semaphore held. The revocation method is called with the target key semaphore held write-locked and the search of the context process's keyrings is done with the auth key semaphore read-locked. The check for the revocation state of the auth key just prior to searching it is done after the auth key is read-locked for the search. This ensures that the auth key can't be revoked between the check and the search. The revocation notification method is added so that the context task_struct can be released as soon as instantiation happens rather than waiting for the auth key to be destroyed, thus avoiding the unnecessary pinning of the requesting process. Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Diffstat (limited to 'security/keys')
-rw-r--r--security/keys/key.c4
-rw-r--r--security/keys/process_keys.c42
-rw-r--r--security/keys/request_key_auth.c45
3 files changed, 74 insertions, 17 deletions
diff --git a/security/keys/key.c b/security/keys/key.c
index 14a15abb7735..51f851557389 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -907,6 +907,10 @@ void key_revoke(struct key *key)
907 * it */ 907 * it */
908 down_write(&key->sem); 908 down_write(&key->sem);
909 set_bit(KEY_FLAG_REVOKED, &key->flags); 909 set_bit(KEY_FLAG_REVOKED, &key->flags);
910
911 if (key->type->revoke)
912 key->type->revoke(key);
913
910 up_write(&key->sem); 914 up_write(&key->sem);
911 915
912} /* end key_revoke() */ 916} /* end key_revoke() */
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index a50a91332fe1..4d9825f9962c 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -391,6 +391,8 @@ key_ref_t search_process_keyrings(struct key_type *type,
391 struct request_key_auth *rka; 391 struct request_key_auth *rka;
392 key_ref_t key_ref, ret, err; 392 key_ref_t key_ref, ret, err;
393 393
394 might_sleep();
395
394 /* we want to return -EAGAIN or -ENOKEY if any of the keyrings were 396 /* we want to return -EAGAIN or -ENOKEY if any of the keyrings were
395 * searchable, but we failed to find a key or we found a negative key; 397 * searchable, but we failed to find a key or we found a negative key;
396 * otherwise we want to return a sample error (probably -EACCES) if 398 * otherwise we want to return a sample error (probably -EACCES) if
@@ -496,27 +498,35 @@ key_ref_t search_process_keyrings(struct key_type *type,
496 */ 498 */
497 if (context->request_key_auth && 499 if (context->request_key_auth &&
498 context == current && 500 context == current &&
499 type != &key_type_request_key_auth && 501 type != &key_type_request_key_auth
500 key_validate(context->request_key_auth) == 0
501 ) { 502 ) {
502 rka = context->request_key_auth->payload.data; 503 /* defend against the auth key being revoked */
504 down_read(&context->request_key_auth->sem);
503 505
504 key_ref = search_process_keyrings(type, description, match, 506 if (key_validate(context->request_key_auth) == 0) {
505 rka->context); 507 rka = context->request_key_auth->payload.data;
506 508
507 if (!IS_ERR(key_ref)) 509 key_ref = search_process_keyrings(type, description,
508 goto found; 510 match, rka->context);
509 511
510 switch (PTR_ERR(key_ref)) { 512 up_read(&context->request_key_auth->sem);
511 case -EAGAIN: /* no key */ 513
512 if (ret) 514 if (!IS_ERR(key_ref))
515 goto found;
516
517 switch (PTR_ERR(key_ref)) {
518 case -EAGAIN: /* no key */
519 if (ret)
520 break;
521 case -ENOKEY: /* negative key */
522 ret = key_ref;
513 break; 523 break;
514 case -ENOKEY: /* negative key */ 524 default:
515 ret = key_ref; 525 err = key_ref;
516 break; 526 break;
517 default: 527 }
518 err = key_ref; 528 } else {
519 break; 529 up_read(&context->request_key_auth->sem);
520 } 530 }
521 } 531 }
522 532
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 0ecc2e8d2bd0..cb9817ced3fd 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -20,6 +20,7 @@
20 20
21static int request_key_auth_instantiate(struct key *, const void *, size_t); 21static int request_key_auth_instantiate(struct key *, const void *, size_t);
22static void request_key_auth_describe(const struct key *, struct seq_file *); 22static void request_key_auth_describe(const struct key *, struct seq_file *);
23static void request_key_auth_revoke(struct key *);
23static void request_key_auth_destroy(struct key *); 24static void request_key_auth_destroy(struct key *);
24static long request_key_auth_read(const struct key *, char __user *, size_t); 25static long request_key_auth_read(const struct key *, char __user *, size_t);
25 26
@@ -31,6 +32,7 @@ struct key_type key_type_request_key_auth = {
31 .def_datalen = sizeof(struct request_key_auth), 32 .def_datalen = sizeof(struct request_key_auth),
32 .instantiate = request_key_auth_instantiate, 33 .instantiate = request_key_auth_instantiate,
33 .describe = request_key_auth_describe, 34 .describe = request_key_auth_describe,
35 .revoke = request_key_auth_revoke,
34 .destroy = request_key_auth_destroy, 36 .destroy = request_key_auth_destroy,
35 .read = request_key_auth_read, 37 .read = request_key_auth_read,
36}; 38};
@@ -93,6 +95,24 @@ static long request_key_auth_read(const struct key *key,
93 95
94/*****************************************************************************/ 96/*****************************************************************************/
95/* 97/*
98 * handle revocation of an authorisation token key
99 * - called with the key sem write-locked
100 */
101static void request_key_auth_revoke(struct key *key)
102{
103 struct request_key_auth *rka = key->payload.data;
104
105 kenter("{%d}", key->serial);
106
107 if (rka->context) {
108 put_task_struct(rka->context);
109 rka->context = NULL;
110 }
111
112} /* end request_key_auth_revoke() */
113
114/*****************************************************************************/
115/*
96 * destroy an instantiation authorisation token key 116 * destroy an instantiation authorisation token key
97 */ 117 */
98static void request_key_auth_destroy(struct key *key) 118static void request_key_auth_destroy(struct key *key)
@@ -101,6 +121,11 @@ static void request_key_auth_destroy(struct key *key)
101 121
102 kenter("{%d}", key->serial); 122 kenter("{%d}", key->serial);
103 123
124 if (rka->context) {
125 put_task_struct(rka->context);
126 rka->context = NULL;
127 }
128
104 key_put(rka->target_key); 129 key_put(rka->target_key);
105 kfree(rka); 130 kfree(rka);
106 131
@@ -131,14 +156,26 @@ struct key *request_key_auth_new(struct key *target, const char *callout_info)
131 * another process */ 156 * another process */
132 if (current->request_key_auth) { 157 if (current->request_key_auth) {
133 /* it is - use that instantiation context here too */ 158 /* it is - use that instantiation context here too */
159 down_read(&current->request_key_auth->sem);
160
161 /* if the auth key has been revoked, then the key we're
162 * servicing is already instantiated */
163 if (test_bit(KEY_FLAG_REVOKED,
164 &current->request_key_auth->flags))
165 goto auth_key_revoked;
166
134 irka = current->request_key_auth->payload.data; 167 irka = current->request_key_auth->payload.data;
135 rka->context = irka->context; 168 rka->context = irka->context;
136 rka->pid = irka->pid; 169 rka->pid = irka->pid;
170 get_task_struct(rka->context);
171
172 up_read(&current->request_key_auth->sem);
137 } 173 }
138 else { 174 else {
139 /* it isn't - use this process as the context */ 175 /* it isn't - use this process as the context */
140 rka->context = current; 176 rka->context = current;
141 rka->pid = current->pid; 177 rka->pid = current->pid;
178 get_task_struct(rka->context);
142 } 179 }
143 180
144 rka->target_key = key_get(target); 181 rka->target_key = key_get(target);
@@ -161,9 +198,15 @@ struct key *request_key_auth_new(struct key *target, const char *callout_info)
161 if (ret < 0) 198 if (ret < 0)
162 goto error_inst; 199 goto error_inst;
163 200
164 kleave(" = {%d})", authkey->serial); 201 kleave(" = {%d}", authkey->serial);
165 return authkey; 202 return authkey;
166 203
204auth_key_revoked:
205 up_read(&current->request_key_auth->sem);
206 kfree(rka);
207 kleave("= -EKEYREVOKED");
208 return ERR_PTR(-EKEYREVOKED);
209
167error_inst: 210error_inst:
168 key_revoke(authkey); 211 key_revoke(authkey);
169 key_put(authkey); 212 key_put(authkey);