linux-stable.git/security/keys/request_key.c, branch linux-2.6.38.y

KEYS: Fix __key_link_end() quota fixup on error

2011-01-25T22:58:20+00:00

Fix __key_link_end()'s attempt to fix up the quota if an error occurs.

There are two erroneous cases: Firstly, we always decrease the quota if
the preallocated replacement keyring needs cleaning up, irrespective of
whether or not we should (we may have replaced a pointer rather than
adding another pointer).

Secondly, we never clean up the quota if we added a pointer without the
keyring storage being extended (we allocate multiple pointers at a time,
even if we're not going to use them all immediately).

We handle this by setting the bottom bit of the preallocation pointer in
__key_link_begin() to indicate that the quota needs fixing up, which is
then passed to __key_link() (which clears the whole thing) and
__key_link_end().

Signed-off-by: David Howells 
Signed-off-by: Linus Torvalds

KEYS: Fix up comments in key management code

2011-01-21T22:59:30+00:00

Fix up comments in the key management code.  No functional changes.

Signed-off-by: David Howells 
Signed-off-by: Linus Torvalds

KEYS: Don't call up_write() if __key_link_begin() returns an error

2010-12-23T23:31:48+00:00

In construct_alloc_key(), up_write() is called in the error path if
__key_link_begin() fails, but this is incorrect as __key_link_begin() only
returns with the nominated keyring locked if it returns successfully.

Without this patch, you might see the following in dmesg:

	=====================================
	[ BUG: bad unlock balance detected! ]
	-------------------------------------
	mount.cifs/5769 is trying to release lock (&key->sem) at:
	[] request_key_and_link+0x263/0x3fc
	but there are no more locks to release!

	other info that might help us debug this:
	3 locks held by mount.cifs/5769:
	 #0:  (&type->s_umount_key#41/1){+.+.+.}, at: [] sget+0x278/0x3e7
	 #1:  (&ret_buf->session_mutex){+.+.+.}, at: [] cifs_get_smb_ses+0x35a/0x443 [cifs]
	 #2:  (root_key_user.cons_lock){+.+.+.}, at: [] request_key_and_link+0x10a/0x3fc

	stack backtrace:
	Pid: 5769, comm: mount.cifs Not tainted 2.6.37-rc6+ #1
	Call Trace:
	 [] ? request_key_and_link+0x263/0x3fc
	 [] print_unlock_inbalance_bug+0xca/0xd5
	 [] lock_release_non_nested+0xc1/0x263
	 [] ? request_key_and_link+0x263/0x3fc
	 [] ? request_key_and_link+0x263/0x3fc
	 [] lock_release+0x17d/0x1a4
	 [] up_write+0x23/0x3b
	 [] request_key_and_link+0x263/0x3fc
	 [] ? cifs_get_spnego_key+0x61/0x21f [cifs]
	 [] request_key+0x41/0x74
	 [] cifs_get_spnego_key+0x200/0x21f [cifs]
	 [] CIFS_SessSetup+0x55d/0x1273 [cifs]
	 [] cifs_setup_session+0x90/0x1ae [cifs]
	 [] cifs_get_smb_ses+0x37f/0x443 [cifs]
	 [] cifs_mount+0x1aa1/0x23f3 [cifs]
	 [] ? alloc_debug_processing+0xdb/0x120
	 [] ? cifs_get_spnego_key+0x1ef/0x21f [cifs]
	 [] cifs_do_mount+0x165/0x2b3 [cifs]
	 [] vfs_kern_mount+0xaf/0x1dc
	 [] do_kern_mount+0x4d/0xef
	 [] do_mount+0x6f4/0x733
	 [] sys_mount+0x88/0xc2
	 [] system_call_fastpath+0x16/0x1b

Reported-by: Jeff Layton 
Signed-off-by: David Howells 
Reviewed-and-Tested-by: Jeff Layton 
Signed-off-by: Linus Torvalds

KEYS: request_key() should return -ENOKEY if the constructed key is negative

2010-08-06T16:17:02+00:00

request_key() should return -ENOKEY if the key it constructs has been
negatively instantiated.

Without this, request_key() can return an unusable key to its caller,
and if the caller then does key_validate() that won't catch the problem.

Signed-off-by: David Howells 
Signed-off-by: Linus Torvalds

KEYS: Reinstate lost passing of process keyring ID in call_sbin_request_key()

2010-08-02T05:34:56+00:00

In commit bb952bb98a7e479262c7eb25d5592545a3af147d there was the accidental
deletion of a statement from call_sbin_request_key() to render the process
keyring ID to a text string so that it can be passed to /sbin/request-key.

With gcc 4.6.0 this causes the following warning:

  CC      security/keys/request_key.o
security/keys/request_key.c: In function 'call_sbin_request_key':
security/keys/request_key.c:102:15: warning: variable 'prkey' set but not used

This patch reinstates that statement.

Without this statement, /sbin/request-key will get some random rubbish from the
stack as that parameter.

Signed-off-by: Justin P. Mattock 
Signed-off-by: David Howells 
Signed-off-by: James Morris

umh: creds: convert call_usermodehelper_keys() to use subprocess_info->init()

2010-05-27T16:12:45+00:00

call_usermodehelper_keys() uses call_usermodehelper_setkeys() to change
subprocess_info->cred in advance.  Now that we have info->init() we can
change this code to set tgcred->session_keyring in context of execing
kernel thread.

Note: since currently call_usermodehelper_keys() is never called with
UMH_NO_WAIT, call_usermodehelper_keys()->key_get() and umh_keys_cleanup()
are not really needed, we could rely on install_session_keyring_to_cred()
which does key_get() on success.

Signed-off-by: Oleg Nesterov 
Acked-by: Neil Horman 
Acked-by: David Howells 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

KEYS: Do preallocation for __key_link()

2010-05-06T12:25:02+00:00

Do preallocation for __key_link() so that the various callers in request_key.c
can deal with any errors from this source before attempting to construct a key.
This allows them to assume that the actual linkage step is guaranteed to be
successful.

Signed-off-by: David Howells 
Signed-off-by: James Morris

Merge branch 'master' into next

2010-05-06T12:21:04+00:00

Conflicts:
	security/keys/keyring.c

Resolved conflict with whitespace fix in find_keyring_by_name()

Signed-off-by: James Morris

KEYS: Better handling of errors from construct_alloc_key()

2010-05-06T00:56:55+00:00

Errors from construct_alloc_key() shouldn't just be ignored in the way they are
by construct_key_and_link().  The only error that can be ignored so is
EINPROGRESS as that is used to indicate that we've found a key and don't need
to construct one.

We don't, however, handle ENOMEM, EDQUOT or EACCES to indicate allocation
failures of one sort or another.

Reported-by: Vegard Nossum 
Signed-off-by: David Howells 
Signed-off-by: James Morris

KEYS: call_sbin_request_key() must write lock keyrings before modifying them

2010-05-05T13:50:24+00:00

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: [] key_type_lookup+0x1c/0x71
	 #1:  (keyring_serialise_link_sem){+.+.+.}, at: [] __key_link+0x4d/0x3c5

	stack backtrace:
	Pid: 2076, comm: keyctl Not tainted 2.6.34-rc6-cachefs #54
	Call Trace:
	 [] lockdep_rcu_dereference+0xaa/0xb2
	 [] ? __key_link+0x4d/0x3c5
	 [] __key_link+0x19e/0x3c5
	 [] ? __key_instantiate_and_link+0xb1/0xdc
	 [] ? key_instantiate_and_link+0x42/0x5f
	 [] call_sbin_request_key+0xe7/0x33b
	 [] ? mutex_unlock+0x9/0xb
	 [] ? __key_instantiate_and_link+0xb1/0xdc
	 [] ? key_instantiate_and_link+0x42/0x5f
	 [] ? request_key_auth_new+0x1c2/0x23c
	 [] ? cache_alloc_debugcheck_after+0x108/0x173
	 [] ? request_key_and_link+0x146/0x300
	 [] ? kmem_cache_alloc+0xe1/0x118
	 [] request_key_and_link+0x28b/0x300
	 [] sys_request_key+0xf7/0x14a
	 [] ? trace_hardirqs_on_caller+0x10c/0x130
	 [] ? trace_hardirqs_on_thunk+0x3a/0x3f
	 [] system_call_fastpath+0x16/0x1b

Signed-off-by: David Howells 
Signed-off-by: James Morris