linux-toradex.git/security/keys, branch v2.6.36.4

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

2011-02-17T22:47:26+00:00

commit 3fc5e98d8cf85e0d77fc597b49e9268dff67400e upstream.

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 
Cc: James Morris 
Signed-off-by: Greg Kroah-Hartman

KEYS: Fix bug in keyctl_session_to_parent() if parent has no session keyring

2010-09-10T14:30:00+00:00

Fix a bug in keyctl_session_to_parent() whereby it tries to check the ownership
of the parent process's session keyring whether or not the parent has a session
keyring [CVE-2010-2960].

This results in the following oops:

  BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0
  IP: [] keyctl_session_to_parent+0x251/0x443
  ...
  Call Trace:
   [] ? keyctl_session_to_parent+0x67/0x443
   [] ? __do_fault+0x24b/0x3d0
   [] sys_keyctl+0xb4/0xb8
   [] system_call_fastpath+0x16/0x1b

if the parent process has no session keyring.

If the system is using pam_keyinit then it mostly protected against this as all
processes derived from a login will have inherited the session keyring created
by pam_keyinit during the log in procedure.

To test this, pam_keyinit calls need to be commented out in /etc/pam.d/.

Reported-by: Tavis Ormandy 
Signed-off-by: David Howells 
Acked-by: Tavis Ormandy 
Signed-off-by: Linus Torvalds

KEYS: Fix RCU no-lock warning in keyctl_session_to_parent()

2010-09-10T14:30:00+00:00

There's an protected access to the parent process's credentials in the middle
of keyctl_session_to_parent().  This results in the following RCU warning:

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

  other info that might help us debug this:

  rcu_scheduler_active = 1, debug_locks = 0
  1 lock held by keyctl-session-/2137:
   #0:  (tasklist_lock){.+.+..}, at: [] keyctl_session_to_parent+0x60/0x236

  stack backtrace:
  Pid: 2137, comm: keyctl-session- Not tainted 2.6.36-rc2-cachefs+ #1
  Call Trace:
   [] lockdep_rcu_dereference+0xaa/0xb3
   [] keyctl_session_to_parent+0xed/0x236
   [] sys_keyctl+0xb4/0xb6
   [] system_call_fastpath+0x16/0x1b

The code should take the RCU read lock to make sure the parents credentials
don't go away, even though it's holding a spinlock and has IRQ disabled.

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

Add a dummy printk function for the maintenance of unused printks

2010-08-12T16:51:35+00:00

Add a dummy printk function for the maintenance of unused printks through gcc
format checking, and also so that side-effect checking is maintained too.

Signed-off-by: David Howells 
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

KEYS: Use the variable 'key' in keyctl_describe_key()

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

keyctl_describe_key() turns the key reference it gets into a usable key pointer
and assigns that to a variable called 'key', which it then ignores in favour of
recomputing the key pointer each time it needs it.  Make it use the precomputed
pointer instead.

Without this patch, gcc 4.6 reports that the variable key is set but not used:

	building with gcc 4.6 I'm getting a warning message:
	 CC      security/keys/keyctl.o
	security/keys/keyctl.c: In function 'keyctl_describe_key':
	security/keys/keyctl.c:472:14: warning: variable 'key' set but not used

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

KEYS: Make /proc/keys check to see if a key is possessed before security check

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

Make /proc/keys check to see if the calling process possesses each key before
performing the security check.  The possession check can be skipped if the key
doesn't have the possessor-view permission bit set.

This causes the keys a process possesses to show up in /proc/keys, even if they
don't have matching user/group/other view permissions.

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

KEYS: Authorise keyctl_set_timeout() on a key if we have its authorisation key

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

Authorise a process to perform keyctl_set_timeout() on an uninstantiated key if
that process has the authorisation key for it.

This allows the instantiator to set the timeout on a key it is instantiating -
provided it does it before instantiating the key.

For instance, the test upcall script provided with the keyutils package could
be modified to set the expiry to an hour hence before instantiating the key:

	[/usr/share/keyutils/request-key-debug.sh]
	 if [ "$3" != "neg" ]
	 then
	+    keyctl timeout $1 3600
	     keyctl instantiate $1 "Debug $3" $4 || exit 1
	 else

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

KEYS: Propagate error code instead of returning -EINVAL

2010-06-27T14:02:34+00:00

This is from a Smatch check I'm writing.

strncpy_from_user() returns -EFAULT on error so the first change just
silences a warning but doesn't change how the code works.

The other change is a bug fix because install_thread_keyring_to_cred()
can return a variety of errors such as -EINVAL, -EEXIST, -ENOMEM or
-EKEYREVOKED.

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