Re: [PATCH 2/3] Keys: Use RCU to manage session keyring pointer

[David Howells]

The attached patch uses RCU to manage the session keyring pointer in struct
signal_struct. This means that searching need not disable interrupts and get a
the sighand spinlock to access this pointer. Furthermore, by judicious use of
rcu_read_(un)lock(), this patch also avoids the need to take and put refcounts
on the session keyring itself, thus saving on even more atomic ops.

Signed-Off-By: David Howells <dhowells@xxxxxxxxxx>
---
warthog>diffstat -p1 keys-rcu-session-2612rc1mm1.diff 
 security/keys/process_keys.c |   42 +++++++++++++++++++++---------------------
 security/keys/request_key.c  |    7 +++----
 2 files changed, 24 insertions(+), 25 deletions(-)

diff -uNrp linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/process_keys.c linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/process_keys.c
--- linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/process_keys.c	2005-03-23 17:22:46.000000000 +0000
+++ linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/process_keys.c	2005-03-23 18:27:12.055768099 +0000
@@ -1,6 +1,6 @@
 /* process_keys.c: management of a process's keyrings
  *
- * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2004-5 Red Hat, Inc. All Rights Reserved.
  * Written by David Howells (dhowells@xxxxxxxxxx)
  *
  * This program is free software; you can redistribute it and/or
@@ -181,7 +181,7 @@ static int install_process_keyring(struc
 			goto error;
 		}
 
-		/* attach or swap keyrings */
+		/* attach keyring */
 		spin_lock_irqsave(&tsk->sighand->siglock, flags);
 		if (!tsk->signal->process_keyring) {
 			tsk->signal->process_keyring = keyring;
@@ -227,12 +227,14 @@ static int install_session_keyring(struc
 
 	/* install the keyring */
 	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-	old = tsk->signal->session_keyring;
-	tsk->signal->session_keyring = keyring;
+	old = rcu_dereference(tsk->signal->session_keyring);
+	rcu_assign_pointer(tsk->signal->session_keyring, keyring);
 	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
 
 	ret = 0;
 
+	/* we're using RCU on the pointer */
+	synchronize_kernel();
 	key_put(old);
  error:
 	return ret;
@@ -245,8 +247,6 @@ static int install_session_keyring(struc
  */
 int copy_thread_group_keys(struct task_struct *tsk)
 {
-	unsigned long flags;
-
 	key_check(current->thread_group->session_keyring);
 	key_check(current->thread_group->process_keyring);
 
@@ -254,10 +254,10 @@ int copy_thread_group_keys(struct task_s
 	tsk->signal->process_keyring = NULL;
 
 	/* same session keyring */
-	spin_lock_irqsave(&current->sighand->siglock, flags);
+	rcu_read_lock();
 	tsk->signal->session_keyring =
-		key_get(current->signal->session_keyring);
-	spin_unlock_irqrestore(&current->sighand->siglock, flags);
+		key_get(rcu_dereference(current->signal->session_keyring));
+	rcu_read_unlock();
 
 	return 0;
 
@@ -381,8 +381,7 @@ struct key *search_process_keyrings_aux(
 					key_match_func_t match)
 {
 	struct task_struct *tsk = current;
-	unsigned long flags;
-	struct key *key, *ret, *err, *tmp;
+	struct key *key, *ret, *err;
 
 	/* 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;
@@ -436,17 +435,18 @@ struct key *search_process_keyrings_aux(
 	}
 
 	/* search the session keyring last */
-	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-
-	tmp = tsk->signal->session_keyring;
-	if (!tmp)
-		tmp = tsk->user->session_keyring;
-	atomic_inc(&tmp->usage);
-
-	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+	if (tsk->signal->session_keyring) {
+		rcu_read_lock();
+		key = keyring_search_aux(
+			rcu_dereference(tsk->signal->session_keyring),
+			type, description, match);
+		rcu_read_unlock();
+	}
+	else {
+		key = keyring_search_aux(tsk->user->session_keyring,
+					 type, description, match);
+	}
 
-	key = keyring_search_aux(tmp, type, description, match);
-	key_put(tmp);
 	if (!IS_ERR(key))
 		goto found;
 
diff -uNrp linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/request_key.c linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/request_key.c
--- linux-2.6.12-rc1-mm1-keys-umhelper/security/keys/request_key.c	2005-03-23 17:35:16.000000000 +0000
+++ linux-2.6.12-rc1-mm1-keys-rcu-session/security/keys/request_key.c	2005-03-23 18:14:13.908029567 +0000
@@ -175,13 +175,12 @@ static struct key *__request_key_constru
 	key->expiry = now.tv_sec + key_negative_timeout;
 
 	if (current->signal->session_keyring) {
-		unsigned long flags;
 		struct key *keyring;
 
-		spin_lock_irqsave(&current->sighand->siglock, flags);
-		keyring = current->signal->session_keyring;
+		rcu_read_lock();
+		keyring = rcu_dereference(current->signal->session_keyring);
 		atomic_inc(&keyring->usage);
-		spin_unlock_irqrestore(&current->sighand->siglock, flags);
+		rcu_read_unlock();
 
 		key_link(keyring, key);
 		key_put(keyring);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/