public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: torvalds@osdl.org, akpm@osdl.org
Cc: keyrings@linux-nfs.org, linux-kernel@vger.kernel.org
Subject: [PATCH] Keys: Improve usage of memory barriers and remove IRQ disablement
Date: Tue, 04 Apr 2006 10:55:30 +0100	[thread overview]
Message-ID: <20060404095529.31311.3892.stgit@warthog.cambridge.redhat.com> (raw)

The attached patch adds a missing memory barrier into the key_put() and removes
an unnecessary barrier 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.
---

 security/keys/key.c          |    1 +
 security/keys/process_keys.c |   41 ++++++++++++++++++++---------------------
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/security/keys/key.c b/security/keys/key.c
index 99781b7..d8a6e00 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -619,6 +619,7 @@ void key_put(struct key *key)
 	if (key) {
 		key_check(key);
 
+		smp_mb__before_atomic_dec();
 		if (atomic_dec_and_test(&key->usage))
 			schedule_work(&key_cleanup_task);
 	}
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 74cb79e..ad123c3 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -167,11 +167,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);
 
@@ -182,12 +183,12 @@ int install_process_keyring(struct task_
 		}
 
 		/* 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);
 	}
@@ -206,38 +207,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() */
 
@@ -310,7 +310,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 */
@@ -322,10 +321,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);
 


             reply	other threads:[~2006-04-04  9:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-04-04  9:55 David Howells [this message]
2006-04-04 10:23 ` [Keyrings] [PATCH] Keys: Improve usage of memory barriers and remove IRQ disablement David Howells
2006-04-04 10:58 ` Nick Piggin
2006-04-05  8:46   ` David Howells
2006-04-05  9:23     ` Nick Piggin
2006-04-05 22:51       ` David S. Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060404095529.31311.3892.stgit@warthog.cambridge.redhat.com \
    --to=dhowells@redhat.com \
    --cc=akpm@osdl.org \
    --cc=keyrings@linux-nfs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox