From 896903c2f5f79f029388f033a00c3b813bc91201 Mon Sep 17 00:00:00 2001
From: David Howells <dhowells@redhat.com>
Date: Fri, 30 Apr 2010 14:32:23 +0100
Subject: [PATCH] KEYS: call_sbin_request_key() must write lock keyrings before
 modifying them

call_sbin_request_key() creates a keyring and then attempts to insert a link to
the authorisation key into that keyring, but does so without holding a write
lock on the keyring semaphore.

It will normally get away with this because it hasn't told anyone that the
keyring exists yet.  The new keyring, however, has had its serial number
published, which means it can be accessed directly by that handle.

This was found by a previous patch that adds RCU lockdep checks to the code
that reads the keyring payload pointer, which includes a check that the keyring
semaphore is actually locked.

Without this patch, the following command:

	keyctl request2 user b a @s

will provoke the following lockdep warning is displayed in dmesg:

	===================================================
	[ INFO: suspicious rcu_dereference_check() usage. ]
	---------------------------------------------------
	security/keys/keyring.c:727 invoked rcu_dereference_check() without protection!

	other info that might help us debug this:

	rcu_scheduler_active = 1, debug_locks = 0
	2 locks held by keyctl/2076:
	 #0:  (key_types_sem){.+.+.+}, at: [<ffffffff811a5b29>] key_type_lookup+0x1c/0x71
	 #1:  (keyring_serialise_link_sem){+.+.+.}, at: [<ffffffff811a6d1e>] __key_link+0x4d/0x3c5

	stack backtrace:
	Pid: 2076, comm: keyctl Not tainted 2.6.34-rc6-cachefs #54
	Call Trace:
	 [<ffffffff81051fdc>] lockdep_rcu_dereference+0xaa/0xb2
	 [<ffffffff811a6d1e>] ? __key_link+0x4d/0x3c5
	 [<ffffffff811a6e6f>] __key_link+0x19e/0x3c5
	 [<ffffffff811a5952>] ? __key_instantiate_and_link+0xb1/0xdc
	 [<ffffffff811a59bf>] ? key_instantiate_and_link+0x42/0x5f
	 [<ffffffff811aa0dc>] call_sbin_request_key+0xe7/0x33b
	 [<ffffffff8139376a>] ? mutex_unlock+0x9/0xb
	 [<ffffffff811a5952>] ? __key_instantiate_and_link+0xb1/0xdc
	 [<ffffffff811a59bf>] ? key_instantiate_and_link+0x42/0x5f
	 [<ffffffff811aa6fa>] ? request_key_auth_new+0x1c2/0x23c
	 [<ffffffff810aaf15>] ? cache_alloc_debugcheck_after+0x108/0x173
	 [<ffffffff811a9d00>] ? request_key_and_link+0x146/0x300
	 [<ffffffff810ac568>] ? kmem_cache_alloc+0xe1/0x118
	 [<ffffffff811a9e45>] request_key_and_link+0x28b/0x300
	 [<ffffffff811a89ac>] sys_request_key+0xf7/0x14a
	 [<ffffffff81052c0b>] ? trace_hardirqs_on_caller+0x10c/0x130
	 [<ffffffff81394fb9>] ? trace_hardirqs_on_thunk+0x3a/0x3f
	 [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
---
 security/keys/request_key.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index d737cea5347..d8c1a6a0fb0 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -94,7 +94,7 @@ static int call_sbin_request_key(struct key_construction *cons,
 	}
 
 	/* attach the auth key to the session keyring */
-	ret = __key_link(keyring, authkey);
+	ret = key_link(keyring, authkey);
 	if (ret < 0)
 		goto error_link;
 
-- 
2.20.1