[PATCH] keys: Discard key spinlock and use RCU for key payload

The attached patch changes the key implementation in a number of ways:

 (1) It removes the spinlock from the key structure.

 (2) The key flags are now accessed using atomic bitops instead of
     write-locking the key spinlock and using C bitwise operators.

     The three instantiation flags are dealt with with the construction
     semaphore held during the request_key/instantiate/negate sequence, thus
     rendering the spinlock superfluous.

     The key flags are also now bit numbers not bit masks.

 (3) The key payload is now accessed using RCU. This permits the recursive
     keyring search algorithm to be simplified greatly since no locks need be
     taken other than the usual RCU preemption disablement. Searching now does
     not require any locks or semaphores to be held; merely that the starting
     keyring be pinned.

 (4) The keyring payload now includes an RCU head so that it can be disposed
     of by call_rcu(). This requires that the payload be copied on unlink to
     prevent introducing races in copy-down vs search-up.

 (5) The user key payload is now a structure with the data following it. It
     includes an RCU head like the keyring payload and for the same reason. It
     also contains a data length because the data length in the key may be
     changed on another CPU whilst an RCU protected read is in progress on the
     payload. This would then see the supposed RCU payload and the on-key data
     length getting out of sync.

     I'm tempted to drop the key's datalen entirely, except that it's used in
     conjunction with quota management and so is a little tricky to get rid
     of.

 (6) Update the keys documentation.

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, 61 insertions, 24 deletions

diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index 8d65b3a28129..c33d3614a0db 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -42,12 +42,19 @@ struct key_type key_type_user = {
         .read           = user_read,
 };
 
+struct user_key_payload {
+        struct rcu_head rcu;            /* RCU destructor */
+        unsigned short  datalen;        /* length of this data */
+        char            data[0];        /* actual data */
+};
+
 /*****************************************************************************/
 /*
  * instantiate a user defined key
  */
 static int user_instantiate(struct key *key, const void *data, size_t datalen)
 {
+        struct user_key_payload *upayload;
         int ret;
 
         ret = -EINVAL;
@@ -58,13 +65,15 @@ static int user_instantiate(struct key *key, const void *data, size_t datalen)
         if (ret < 0)
                 goto error;
 
-        /* attach the data */
         ret = -ENOMEM;
-        key->payload.data = kmalloc(datalen, GFP_KERNEL);
-        if (!key->payload.data)
+        upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
+        if (!upayload)
                 goto error;
 
-        memcpy(key->payload.data, data, datalen);
+        /* attach the data */
+        upayload->datalen = datalen;
+        memcpy(upayload->data, data, datalen);
+        rcu_assign_pointer(key->payload.data, upayload);
         ret = 0;
 
  error:
@@ -75,18 +84,25 @@ static int user_instantiate(struct key *key, const void *data, size_t datalen)
 /*****************************************************************************/
 /*
  * duplicate a user defined key
+ * - both keys' semaphores are locked against further modification
+ * - the new key cannot yet be accessed
  */
 static int user_duplicate(struct key *key, const struct key *source)
 {
+        struct user_key_payload *upayload, *spayload;
         int ret;
 
         /* just copy the payload */
         ret = -ENOMEM;
-        key->payload.data = kmalloc(source->datalen, GFP_KERNEL);
+        upayload = kmalloc(sizeof(*upayload) + source->datalen, GFP_KERNEL);
+        if (upayload) {
+                spayload = rcu_dereference(source->payload.data);
+                BUG_ON(source->datalen != spayload->datalen);
 
-        if (key->payload.data) {
-                key->datalen = source->datalen;
-                memcpy(key->payload.data, source->payload.data, source->datalen);
+                upayload->datalen = key->datalen = spayload->datalen;
+                memcpy(upayload->data, spayload->data, key->datalen);
+
+                key->payload.data = upayload;
                 ret = 0;
         }
 
@@ -96,40 +112,54 @@ static int user_duplicate(struct key *key, const struct key *source)
 
 /*****************************************************************************/
 /*
+ * dispose of the old data from an updated user defined key
+ */
+static void user_update_rcu_disposal(struct rcu_head *rcu)
+{
+        struct user_key_payload *upayload;
+
+        upayload = container_of(rcu, struct user_key_payload, rcu);
+
+        kfree(upayload);
+
+} /* end user_update_rcu_disposal() */
+
+/*****************************************************************************/
+/*
  * update a user defined key
+ * - the key's semaphore is write-locked
  */
 static int user_update(struct key *key, const void *data, size_t datalen)
 {
-        void *new, *zap;
+        struct user_key_payload *upayload, *zap;
         int ret;
 
         ret = -EINVAL;
         if (datalen <= 0 || datalen > 32767 || !data)
                 goto error;
 
-        /* copy the data */
+        /* construct a replacement payload */
         ret = -ENOMEM;
-        new = kmalloc(datalen, GFP_KERNEL);
-        if (!new)
+        upayload = kmalloc(sizeof(*upayload) + datalen, GFP_KERNEL);
+        if (!upayload)
                 goto error;
 
-        memcpy(new, data, datalen);
+        upayload->datalen = datalen;
+        memcpy(upayload->data, data, datalen);
 
         /* check the quota and attach the new data */
-        zap = new;
-        write_lock(&key->lock);
+        zap = upayload;
 
         ret = key_payload_reserve(key, datalen);
 
         if (ret == 0) {
                 /* attach the new data, displacing the old */
                 zap = key->payload.data;
-                key->payload.data = new;
+                rcu_assign_pointer(key->payload.data, upayload);
                 key->expiry = 0;
         }
 
-        write_unlock(&key->lock);
-        kfree(zap);
+        call_rcu(&zap->rcu, user_update_rcu_disposal);
 
  error:
         return ret;
@@ -152,13 +182,15 @@ static int user_match(const struct key *key, const void *description)
  */
 static void user_destroy(struct key *key)
 {
-        kfree(key->payload.data);
+        struct user_key_payload *upayload = key->payload.data;
+
+        kfree(upayload);
 
 } /* end user_destroy() */
 
 /*****************************************************************************/
 /*
- * describe the user
+ * describe the user key
  */
 static void user_describe(const struct key *key, struct seq_file *m)
 {
@@ -171,18 +203,23 @@ static void user_describe(const struct key *key, struct seq_file *m)
 /*****************************************************************************/
 /*
  * read the key data
+ * - the key's semaphore is read-locked
  */
 static long user_read(const struct key *key,
                       char __user *buffer, size_t buflen)
 {
-        long ret = key->datalen;
+        struct user_key_payload *upayload;
+        long ret;
+
+        upayload = rcu_dereference(key->payload.data);
+        ret = upayload->datalen;
 
         /* we can return the data as is */
         if (buffer && buflen > 0) {
-                if (buflen > key->datalen)
-                        buflen = key->datalen;
+                if (buflen > upayload->datalen)
+                        buflen = upayload->datalen;
 
-                if (copy_to_user(buffer, key->payload.data, buflen) != 0)
+                if (copy_to_user(buffer, upayload->data, buflen) != 0)
                         ret = -EFAULT;
         }