[PATCH] Keys: Improve usage of memory barriers and remove IRQ disablement

Remove an unnecessary memory barrier (implicit in rcu_dereference()) from
install_session_keyring().

install_session_keyring() is also rearranged a little to make it slightly
more efficient.

As install_*_keyring() may schedule (in synchronize_rcu() or
keyring_alloc()), they may not be entered with interrupts disabled - and so
there's no point saving the interrupt disablement state over the critical
section.

exec_keys() will also be invoked with interrupts enabled, and so that doesn't
need to save the interrupt state either.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index f6940618e345796a90ed32a30ec9a53024862090..217a0bef3c82b341db5c07d414b7385a8f4a00fb 100644 (file)
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -168,11 +168,12 @@ error:
  */
 int install_process_keyring(struct task_struct *tsk)
 {
-       unsigned long flags;
        struct key *keyring;
        char buf[20];
        int ret;
 
+       might_sleep();
+
        if (!tsk->signal->process_keyring) {
                sprintf(buf, "_pid.%u", tsk->tgid);
 
@@ -183,12 +184,12 @@ int install_process_keyring(struct task_struct *tsk)
                }
 
                /* attach keyring */
-               spin_lock_irqsave(&tsk->sighand->siglock, flags);
+               spin_lock_irq(&tsk->sighand->siglock);
                if (!tsk->signal->process_keyring) {
                        tsk->signal->process_keyring = keyring;
                        keyring = NULL;
                }
-               spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+               spin_unlock_irq(&tsk->sighand->siglock);
 
                key_put(keyring);
        }
@@ -207,38 +208,37 @@ error:
 static int install_session_keyring(struct task_struct *tsk,
                                   struct key *keyring)
 {
-       unsigned long flags;
        struct key *old;
        char buf[20];
-       int ret;
+
+       might_sleep();
 
        /* create an empty session keyring */
        if (!keyring) {
                sprintf(buf, "_ses.%u", tsk->tgid);
 
                keyring = keyring_alloc(buf, tsk->uid, tsk->gid, 1, NULL);
-               if (IS_ERR(keyring)) {
-                       ret = PTR_ERR(keyring);
-                       goto error;
-               }
+               if (IS_ERR(keyring))
+                       return PTR_ERR(keyring);
        }
        else {
                atomic_inc(&keyring->usage);
        }
 
        /* install the keyring */
-       spin_lock_irqsave(&tsk->sighand->siglock, flags);
-       old = rcu_dereference(tsk->signal->session_keyring);
+       spin_lock_irq(&tsk->sighand->siglock);
+       old = tsk->signal->session_keyring;
        rcu_assign_pointer(tsk->signal->session_keyring, keyring);
-       spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+       spin_unlock_irq(&tsk->sighand->siglock);
 
-       ret = 0;
+       /* we're using RCU on the pointer, but there's no point synchronising
+        * on it if it didn't previously point to anything */
+       if (old) {
+               synchronize_rcu();
+               key_put(old);
+       }
 
-       /* we're using RCU on the pointer */
-       synchronize_rcu();
-       key_put(old);
-error:
-       return ret;
+       return 0;
 
 } /* end install_session_keyring() */
 
@@ -311,7 +311,6 @@ void exit_keys(struct task_struct *tsk)
  */
 int exec_keys(struct task_struct *tsk)
 {
-       unsigned long flags;
        struct key *old;
 
        /* newly exec'd tasks don't get a thread keyring */
@@ -323,10 +322,10 @@ int exec_keys(struct task_struct *tsk)
        key_put(old);
 
        /* discard the process keyring from a newly exec'd task */
-       spin_lock_irqsave(&tsk->sighand->siglock, flags);
+       spin_lock_irq(&tsk->sighand->siglock);
        old = tsk->signal->process_keyring;
        tsk->signal->process_keyring = NULL;
-       spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+       spin_unlock_irq(&tsk->sighand->siglock);
 
        key_put(old);