[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>

1 files changed, 26 insertions, 16 deletions

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,
         struct request_key_auth *rka;
         key_ref_t key_ref, ret, err;
 
+        might_sleep();
+
         /* we want to return -EAGAIN or -ENOKEY if any of the keyrings were
          * searchable, but we failed to find a key or we found a negative key;
          * 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,
          */
         if (context->request_key_auth &&
             context == current &&
-            type != &key_type_request_key_auth &&
-            key_validate(context->request_key_auth) == 0
+            type != &key_type_request_key_auth
             ) {
-                rka = context->request_key_auth->payload.data;
+                /* defend against the auth key being revoked */
+                down_read(&context->request_key_auth->sem);
 
-                key_ref = search_process_keyrings(type, description, match,
-                                                  rka->context);
+                if (key_validate(context->request_key_auth) == 0) {
+                        rka = context->request_key_auth->payload.data;
 
-                if (!IS_ERR(key_ref))
-                        goto found;
+                        key_ref = search_process_keyrings(type, description,
+                                                          match, rka->context);
 
-                switch (PTR_ERR(key_ref)) {
-                case -EAGAIN: /* no key */
-                        if (ret)
+                        up_read(&context->request_key_auth->sem);
+
+                        if (!IS_ERR(key_ref))
+                                goto found;
+
+                        switch (PTR_ERR(key_ref)) {
+                        case -EAGAIN: /* no key */
+                                if (ret)
+                                        break;
+                        case -ENOKEY: /* negative key */
+                                ret = key_ref;
                                 break;
-                case -ENOKEY: /* negative key */
-                        ret = key_ref;
-                        break;
-                default:
-                        err = key_ref;
-                        break;
+                        default:
+                                err = key_ref;
+                                break;
+                        }
+                } else {
+                        up_read(&context->request_key_auth->sem);
                 }
         }