Commit graph

163 commits

Author SHA1 Message Date
Lai Jiangshan
3acb458c32 security,rcu: convert call_rcu(user_update_rcu_disposal) to kfree_rcu()
The rcu callback user_update_rcu_disposal() just calls a kfree(),
so we use kfree_rcu() instead of the call_rcu(user_update_rcu_disposal).

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
2011-05-07 22:50:54 -07:00
David Howells
ee009e4a0d KEYS: Add an iovec version of KEYCTL_INSTANTIATE
Add a keyctl op (KEYCTL_INSTANTIATE_IOV) that is like KEYCTL_INSTANTIATE, but
takes an iovec array and concatenates the data in-kernel into one buffer.
Since the KEYCTL_INSTANTIATE copies the data anyway, this isn't too much of a
problem.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2011-03-08 11:17:22 +11:00
David Howells
fdd1b94581 KEYS: Add a new keyctl op to reject a key with a specified error code
Add a new keyctl op to reject a key with a specified error code.  This works
much the same as negating a key, and so keyctl_negate_key() is made a special
case of keyctl_reject_key().  The difference is that keyctl_negate_key()
selects ENOKEY as the error to be reported.

Typically the key would be rejected with EKEYEXPIRED, EKEYREVOKED or
EKEYREJECTED, but this is not mandatory.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2011-03-08 11:17:18 +11:00
David Howells
b9fffa3877 KEYS: Add a key type op to permit the key description to be vetted
Add a key type operation to permit the key type to vet the description of a new
key that key_alloc() is about to allocate.  The operation may reject the
description if it wishes with an error of its choosing.  If it does this, the
key will not be allocated.

Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
2011-03-08 11:17:15 +11:00
David Howells
633e804e89 KEYS: Add an RCU payload dereference macro
Add an RCU payload dereference macro as this seems to be a common piece of code
amongst key types that use RCU referenced payloads.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
2011-03-08 11:17:11 +11:00
David Howells
ceb73c1204 KEYS: Fix __key_link_end() quota fixup on error
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 <dhowells@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2011-01-26 08:58:20 +10:00
Jesper Juhl
5403110943 trusted keys: Fix a memory leak in trusted_update().
One failure path in security/keys/trusted.c::trusted_update() does
not free 'new_p' while the others do. This patch makes sure we also free
it in the remaining path (if datablob_parse() returns different from
Opt_update).

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
Signed-off-by: James Morris <jmorris@namei.org>
2011-01-24 10:59:58 +11:00
Mimi Zohar
b970344934 encrypted-keys: rename encrypted_defined files to encrypted
Rename encrypted_defined.c and encrypted_defined.h files to encrypted.c and
encrypted.h, respectively. Based on request from David Howells.

Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2011-01-24 10:27:57 +11:00
Mimi Zohar
4b174b6d28 trusted-keys: rename trusted_defined files to trusted
Rename trusted_defined.c and trusted_defined.h files to trusted.c and
trusted.h, respectively. Based on request from David Howells.

Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2011-01-24 10:14:22 +11:00
David Howells
973c9f4f49 KEYS: Fix up comments in key management code
Fix up comments in the key management code.  No functional changes.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2011-01-21 14:59:30 -08:00
David Howells
a8b17ed019 KEYS: Do some style cleanup in the key management code.
Do a bit of a style clean up in the key management code.  No functional
changes.

Done using:

  perl -p -i -e 's!^/[*]*/\n!!' security/keys/*.c
  perl -p -i -e 's!} /[*] end [a-z0-9_]*[(][)] [*]/\n!}\n!' security/keys/*.c
  sed -i -s -e ": next" -e N -e 's/^\n[}]$/}/' -e t -e P -e 's/^.*\n//' -e "b next" security/keys/*.c

To remove /*****/ lines, remove comments on the closing brace of a
function to name the function and remove blank lines before the closing
brace of a function.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2011-01-21 14:59:29 -08:00
Tetsuo Handa
154a96bfcd trusted-keys: avoid scattring va_end()
We can avoid scattering va_end() within the

  va_start();
  for (;;) {

  }
  va_end();

loop, assuming that crypto_shash_init()/crypto_shash_update() return 0 on
success and negative value otherwise.

Make TSS_authhmac()/TSS_checkhmac1()/TSS_checkhmac2() similar to TSS_rawhmac()
by removing "va_end()/goto" from the loop.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Jesper Juhl <jj@chaosbits.net>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2011-01-19 09:53:59 +11:00
Tetsuo Handa
0e7491f685 trusted-keys: check for NULL before using it
TSS_rawhmac() checks for data != NULL before using it.
We should do the same thing for TSS_authhmac().

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Jesper Juhl <jj@chaosbits.net>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2011-01-19 09:53:56 +11:00
Tetsuo Handa
35576eab39 trusted-keys: another free memory bugfix
TSS_rawhmac() forgot to call va_end()/kfree() when data == NULL and
forgot to call va_end() when crypto_shash_update() < 0.
Fix these bugs by escaping from the loop using "break"
(rather than "return"/"goto") in order to make sure that
va_end()/kfree() are always called.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Jesper Juhl <jj@chaosbits.net>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2011-01-19 09:53:53 +11:00
Mimi Zohar
40c1001792 trusted-keys: free memory bugfix
Add missing kfree(td) in tpm_seal() before the return, freeing
td on error paths as well.

Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: David Safford <safford@watson.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: James Morris <jmorris@namei.org>
2011-01-14 10:27:46 +11:00
James Morris
d2e7ad1922 Merge branch 'master' into next
Conflicts:
	security/smack/smack_lsm.c

Verified and added fix by Stephen Rothwell <sfr@canb.auug.org.au>
Ok'd by Casey Schaufler <casey@schaufler-ca.com>

Signed-off-by: James Morris <jmorris@namei.org>
2011-01-10 09:46:24 +11:00
David Howells
3fc5e98d8c KEYS: Don't call up_write() if __key_link_begin() returns an error
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:
	[<ffffffff81201159>] 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: [<ffffffff81131321>] sget+0x278/0x3e7
	 #1:  (&ret_buf->session_mutex){+.+.+.}, at: [<ffffffffa0258e59>] cifs_get_smb_ses+0x35a/0x443 [cifs]
	 #2:  (root_key_user.cons_lock){+.+.+.}, at: [<ffffffff81201000>] request_key_and_link+0x10a/0x3fc

	stack backtrace:
	Pid: 5769, comm: mount.cifs Not tainted 2.6.37-rc6+ #1
	Call Trace:
	 [<ffffffff81201159>] ? request_key_and_link+0x263/0x3fc
	 [<ffffffff81081601>] print_unlock_inbalance_bug+0xca/0xd5
	 [<ffffffff81083248>] lock_release_non_nested+0xc1/0x263
	 [<ffffffff81201159>] ? request_key_and_link+0x263/0x3fc
	 [<ffffffff81201159>] ? request_key_and_link+0x263/0x3fc
	 [<ffffffff81083567>] lock_release+0x17d/0x1a4
	 [<ffffffff81073f45>] up_write+0x23/0x3b
	 [<ffffffff81201159>] request_key_and_link+0x263/0x3fc
	 [<ffffffffa026fe9e>] ? cifs_get_spnego_key+0x61/0x21f [cifs]
	 [<ffffffff812013c5>] request_key+0x41/0x74
	 [<ffffffffa027003d>] cifs_get_spnego_key+0x200/0x21f [cifs]
	 [<ffffffffa026e296>] CIFS_SessSetup+0x55d/0x1273 [cifs]
	 [<ffffffffa02589e1>] cifs_setup_session+0x90/0x1ae [cifs]
	 [<ffffffffa0258e7e>] cifs_get_smb_ses+0x37f/0x443 [cifs]
	 [<ffffffffa025a9e3>] cifs_mount+0x1aa1/0x23f3 [cifs]
	 [<ffffffff8111fd94>] ? alloc_debug_processing+0xdb/0x120
	 [<ffffffffa027002c>] ? cifs_get_spnego_key+0x1ef/0x21f [cifs]
	 [<ffffffffa024cc71>] cifs_do_mount+0x165/0x2b3 [cifs]
	 [<ffffffff81130e72>] vfs_kern_mount+0xaf/0x1dc
	 [<ffffffff81131007>] do_kern_mount+0x4d/0xef
	 [<ffffffff811483b9>] do_mount+0x6f4/0x733
	 [<ffffffff8114861f>] sys_mount+0x88/0xc2
	 [<ffffffff8100ac42>] system_call_fastpath+0x16/0x1b

Reported-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-and-Tested-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-12-23 15:31:48 -08:00
Mimi Zohar
3b1826cebe encrypted-keys: style and other cleanup
Cleanup based on David Howells suggestions:
- use static const char arrays instead of #define
- rename init_sdesc to alloc_sdesc
- convert 'unsigned int' definitions to 'size_t'
- revert remaining 'const unsigned int' definitions to 'unsigned int'

Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-12-15 12:14:34 +05:30
Mimi Zohar
1f35065a9e encrypted-keys: verify datablob size before converting to binary
Verify the hex ascii datablob length is correct before converting the IV,
encrypted data, and HMAC to binary.

Reported-by: David Howells <dhowells@redhat.com>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-12-15 12:14:32 +05:30
Mimi Zohar
1bdbb4024c trusted-keys: kzalloc and other cleanup
Cleanup based on David Howells suggestions:
- replace kzalloc, where possible, with kmalloc
- revert 'const unsigned int' definitions to 'unsigned int'

Signed-off-by: David Safford <safford@watson.ibm.com>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-12-15 12:14:27 +05:30
Mimi Zohar
bc5e0af0b3 trusted-keys: additional TSS return code and other error handling
Previously not all TSS return codes were tested, as they were all eventually
caught by the TPM. Now all returns are tested and handled immediately.

This patch also fixes memory leaks in error and non-error paths.

Signed-off-by: David Safford <safford@watson.ibm.com>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
Acked-by: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-12-15 12:14:25 +05:30
Mimi Zohar
93ae86e759 keys: add missing include file for trusted and encrypted keys
This patch fixes the linux-next powerpc build errors as reported by
Stephen Rothwell.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Tested-by: Rajiv Andrade <srajiv@linux.vnet.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-11-30 09:20:27 +11:00
Mimi Zohar
7e70cb4978 keys: add new key-type encrypted
Define a new kernel key-type called 'encrypted'. Encrypted keys are kernel
generated random numbers, which are encrypted/decrypted with a 'trusted'
symmetric key. Encrypted keys are created/encrypted/decrypted in the kernel.
Userspace only ever sees/stores encrypted blobs.

Changelog:
- bug fix: replaced master-key rcu based locking with semaphore
  (reported by David Howells)
- Removed memset of crypto_shash_digest() digest output
- Replaced verification of 'key-type:key-desc' using strcspn(), with
  one based on string constants.
- Moved documentation to Documentation/keys-trusted-encrypted.txt
- Replace hash with shash (based on comments by David Howells)
- Make lengths/counts size_t where possible (based on comments by David Howells)
  Could not convert most lengths, as crypto expects 'unsigned int'
  (size_t: on 32 bit is defined as unsigned int, but on 64 bit is unsigned long)
- Add 'const' where possible (based on comments by David Howells)
- allocate derived_buf dynamically to support arbitrary length master key
  (fixed by Roberto Sassu)
- wait until late_initcall for crypto libraries to be registered
- cleanup security/Kconfig
- Add missing 'update' keyword (reported/fixed by Roberto Sassu)
- Free epayload on failure to create key (reported/fixed by Roberto Sassu)
- Increase the data size limit (requested by Roberto Sassu)
- Crypto return codes are always 0 on success and negative on failure,
  remove unnecessary tests.
- Replaced kzalloc() with kmalloc()

Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: David Safford <safford@watson.ibm.com>
Reviewed-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: James Morris <jmorris@namei.org>
2010-11-29 08:55:29 +11:00
Mimi Zohar
d00a1c72f7 keys: add new trusted key-type
Define a new kernel key-type called 'trusted'.  Trusted keys are random
number symmetric keys, generated and RSA-sealed by the TPM.  The TPM
only unseals the keys, if the boot PCRs and other criteria match.
Userspace can only ever see encrypted blobs.

Based on suggestions by Jason Gunthorpe, several new options have been
added to support additional usages.

The new options are:
migratable=  designates that the key may/may not ever be updated
             (resealed under a new key, new pcrinfo or new auth.)

pcrlock=n    extends the designated PCR 'n' with a random value,
             so that a key sealed to that PCR may not be unsealed
             again until after a reboot.

keyhandle=   specifies the sealing/unsealing key handle.

keyauth=     specifies the sealing/unsealing key auth.

blobauth=    specifies the sealed data auth.

Implementation of a kernel reserved locality for trusted keys will be
investigated for a possible future extension.

Changelog:
- Updated and added examples to Documentation/keys-trusted-encrypted.txt
- Moved generic TPM constants to include/linux/tpm_command.h
  (David Howell's suggestion.)
- trusted_defined.c: replaced kzalloc with kmalloc, added pcrlock failure
  error handling, added const qualifiers where appropriate.
- moved to late_initcall
- updated from hash to shash (suggestion by David Howells)
- reduced worst stack usage (tpm_seal) from 530 to 312 bytes
- moved documentation to Documentation directory (suggestion by David Howells)
- all the other code cleanups suggested by David Howells
- Add pcrlock CAP_SYS_ADMIN dependency (based on comment by Jason Gunthorpe)
- New options: migratable, pcrlock, keyhandle, keyauth, blobauth (based on
  discussions with Jason Gunthorpe)
- Free payload on failure to create key(reported/fixed by Roberto Sassu)
- Updated Kconfig and other descriptions (based on Serge Hallyn's suggestion)
- Replaced kzalloc() with kmalloc() (reported by Serge Hallyn)

Signed-off-by: David Safford <safford@watson.ibm.com>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-11-29 08:55:25 +11:00
Andi Kleen
27d6379894 Fix install_process_keyring error handling
Fix an incorrect error check that returns 1 for error instead of the
expected error code.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-10-28 09:02:15 -07:00
David Howells
3d96406c7d KEYS: Fix bug in keyctl_session_to_parent() if parent has no session keyring
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: [<ffffffff811ae4dd>] keyctl_session_to_parent+0x251/0x443
  ...
  Call Trace:
   [<ffffffff811ae2f3>] ? keyctl_session_to_parent+0x67/0x443
   [<ffffffff8109d286>] ? __do_fault+0x24b/0x3d0
   [<ffffffff811af98c>] sys_keyctl+0xb4/0xb8
   [<ffffffff81001eab>] 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 <taviso@cmpxchg8b.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Tavis Ormandy <taviso@cmpxchg8b.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-09-10 07:30:00 -07:00
David Howells
9d1ac65a96 KEYS: Fix RCU no-lock warning in keyctl_session_to_parent()
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: [<ffffffff811ae2ec>] keyctl_session_to_parent+0x60/0x236

  stack backtrace:
  Pid: 2137, comm: keyctl-session- Not tainted 2.6.36-rc2-cachefs+ #1
  Call Trace:
   [<ffffffff8105606a>] lockdep_rcu_dereference+0xaa/0xb3
   [<ffffffff811ae379>] keyctl_session_to_parent+0xed/0x236
   [<ffffffff811af77e>] sys_keyctl+0xb4/0xb6
   [<ffffffff81001eab>] 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 <dhowells@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-09-10 07:30:00 -07:00
David Howells
12fdff3fc2 Add a dummy printk function for the maintenance of unused printks
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 <dhowells@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-08-12 09:51:35 -07:00
David Howells
1e456a1243 KEYS: request_key() should return -ENOKEY if the constructed key is negative
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 <dhowells@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-08-06 09:17:02 -07:00
Justin P. Mattock
5ad18a0d59 KEYS: Reinstate lost passing of process keyring ID in call_sbin_request_key()
In commit bb952bb98a 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 <justinmattock@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-08-02 15:34:56 +10:00
David Howells
94fd8405ea KEYS: Use the variable 'key' in keyctl_describe_key()
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 <justinmattock@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-08-02 15:34:56 +10:00
David Howells
927942aabb KEYS: Make /proc/keys check to see if a key is possessed before security check
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 <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-08-02 15:34:27 +10:00
David Howells
9156235b34 KEYS: Authorise keyctl_set_timeout() on a key if we have its authorisation key
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 <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-08-02 15:34:27 +10:00
Dan Carpenter
4303ef19c6 KEYS: Propagate error code instead of returning -EINVAL
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 <error27@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-06-27 07:02:34 -07:00
Oleg Nesterov
dd98acf747 keyctl_session_to_parent(): use thread_group_empty() to check singlethreadness
No functional changes.

keyctl_session_to_parent() is the only user of signal->count which needs
the correct value.  Change it to use thread_group_empty() instead, this
must be strictly equivalent under tasklist, and imho looks better.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: David Howells <dhowells@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Acked-by: Roland McGrath <roland@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-05-27 09:12:47 -07:00
Oleg Nesterov
685bfd2c48 umh: creds: convert call_usermodehelper_keys() to use subprocess_info->init()
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 <oleg@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-05-27 09:12:45 -07:00
Alexey Dobriyan
4be929be34 kernel-wide: replace USHORT_MAX, SHORT_MAX and SHORT_MIN with USHRT_MAX, SHRT_MAX and SHRT_MIN
- C99 knows about USHRT_MAX/SHRT_MAX/SHRT_MIN, not
  USHORT_MAX/SHORT_MAX/SHORT_MIN.

- Make SHRT_MIN of type s16, not int, for consistency.

[akpm@linux-foundation.org: fix drivers/dma/timb_dma.c]
[akpm@linux-foundation.org: fix security/keys/keyring.c]
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Acked-by: WANG Cong <xiyou.wangcong@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-05-25 08:07:02 -07:00
Dan Carpenter
4d09ec0f70 KEYS: Return more accurate error codes
We were using the wrong variable here so the error codes weren't being returned
properly.  The original code returns -ENOKEY.

Signed-off-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-05-18 08:50:55 +10:00
David Howells
f70e2e0619 KEYS: Do preallocation for __key_link()
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 <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-05-06 22:25:02 +10:00
James Morris
043b4d40f5 Merge branch 'master' into next
Conflicts:
	security/keys/keyring.c

Resolved conflict with whitespace fix in find_keyring_by_name()

Signed-off-by: James Morris <jmorris@namei.org>
2010-05-06 22:21:04 +10:00
David Howells
2b9e4688fa KEYS: Better handling of errors from construct_alloc_key()
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 <vegard.nossum@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-05-06 10:56:55 +10:00
David Howells
553d603c8f KEYS: keyring_serialise_link_sem is only needed for keyring->keyring links
keyring_serialise_link_sem is only needed for keyring->keyring links as it's
used to prevent cycle detection from being avoided by parallel keyring
additions.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-05-06 10:56:52 +10:00
James Morris
0ffbe2699c Merge branch 'master' into next 2010-05-06 10:56:07 +10:00
David Howells
896903c2f5 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>
2010-05-05 23:50:24 +10:00
David Howells
f0641cba77 KEYS: Use RCU dereference wrappers in keyring key type code
The keyring key type code should use RCU dereference wrappers, even when it
holds the keyring's key semaphore.

Reported-by: Vegard Nossum <vegard.nossum@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-05-05 23:50:12 +10:00
Toshiyuki Okajima
cea7daa358 KEYS: find_keyring_by_name() can gain access to a freed keyring
find_keyring_by_name() can gain access to a keyring that has had its reference
count reduced to zero, and is thus ready to be freed.  This then allows the
dead keyring to be brought back into use whilst it is being destroyed.

The following timeline illustrates the process:

|(cleaner)                           (user)
|
| free_user(user)                    sys_keyctl()
|  |                                  |
|  key_put(user->session_keyring)     keyctl_get_keyring_ID()
|  ||	//=> keyring->usage = 0        |
|  |schedule_work(&key_cleanup_task)   lookup_user_key()
|  ||                                   |
|  kmem_cache_free(,user)               |
|  .                                    |[KEY_SPEC_USER_KEYRING]
|  .                                    install_user_keyrings()
|  .                                    ||
| key_cleanup() [<= worker_thread()]    ||
|  |                                    ||
|  [spin_lock(&key_serial_lock)]        |[mutex_lock(&key_user_keyr..mutex)]
|  |                                    ||
|  atomic_read() == 0                   ||
|  |{ rb_ease(&key->serial_node,) }     ||
|  |                                    ||
|  [spin_unlock(&key_serial_lock)]      |find_keyring_by_name()
|  |                                    |||
|  keyring_destroy(keyring)             ||[read_lock(&keyring_name_lock)]
|  ||                                   |||
|  |[write_lock(&keyring_name_lock)]    ||atomic_inc(&keyring->usage)
|  |.                                   ||| *** GET freeing keyring ***
|  |.                                   ||[read_unlock(&keyring_name_lock)]
|  ||                                   ||
|  |list_del()                          |[mutex_unlock(&key_user_k..mutex)]
|  ||                                   |
|  |[write_unlock(&keyring_name_lock)]  ** INVALID keyring is returned **
|  |                                    .
|  kmem_cache_free(,keyring)            .
|                                       .
|                                       atomic_dec(&keyring->usage)
v                                         *** DESTROYED ***
TIME

If CONFIG_SLUB_DEBUG=y then we may see the following message generated:

	=============================================================================
	BUG key_jar: Poison overwritten
	-----------------------------------------------------------------------------

	INFO: 0xffff880197a7e200-0xffff880197a7e200. First byte 0x6a instead of 0x6b
	INFO: Allocated in key_alloc+0x10b/0x35f age=25 cpu=1 pid=5086
	INFO: Freed in key_cleanup+0xd0/0xd5 age=12 cpu=1 pid=10
	INFO: Slab 0xffffea000592cb90 objects=16 used=2 fp=0xffff880197a7e200 flags=0x200000000000c3
	INFO: Object 0xffff880197a7e200 @offset=512 fp=0xffff880197a7e300

	Bytes b4 0xffff880197a7e1f0:  5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
	  Object 0xffff880197a7e200:  6a 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b jkkkkkkkkkkkkkkk

Alternatively, we may see a system panic happen, such as:

	BUG: unable to handle kernel NULL pointer dereference at 0000000000000001
	IP: [<ffffffff810e61a3>] kmem_cache_alloc+0x5b/0xe9
	PGD 6b2b4067 PUD 6a80d067 PMD 0
	Oops: 0000 [#1] SMP
	last sysfs file: /sys/kernel/kexec_crash_loaded
	CPU 1
	...
	Pid: 31245, comm: su Not tainted 2.6.34-rc5-nofixed-nodebug #2 D2089/PRIMERGY
	RIP: 0010:[<ffffffff810e61a3>]  [<ffffffff810e61a3>] kmem_cache_alloc+0x5b/0xe9
	RSP: 0018:ffff88006af3bd98  EFLAGS: 00010002
	RAX: 0000000000000000 RBX: 0000000000000001 RCX: ffff88007d19900b
	RDX: 0000000100000000 RSI: 00000000000080d0 RDI: ffffffff81828430
	RBP: ffffffff81828430 R08: ffff88000a293750 R09: 0000000000000000
	R10: 0000000000000001 R11: 0000000000100000 R12: 00000000000080d0
	R13: 00000000000080d0 R14: 0000000000000296 R15: ffffffff810f20ce
	FS:  00007f97116bc700(0000) GS:ffff88000a280000(0000) knlGS:0000000000000000
	CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
	CR2: 0000000000000001 CR3: 000000006a91c000 CR4: 00000000000006e0
	DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
	DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
	Process su (pid: 31245, threadinfo ffff88006af3a000, task ffff8800374414c0)
	Stack:
	 0000000512e0958e 0000000000008000 ffff880037f8d180 0000000000000001
	 0000000000000000 0000000000008001 ffff88007d199000 ffffffff810f20ce
	 0000000000008000 ffff88006af3be48 0000000000000024 ffffffff810face3
	Call Trace:
	 [<ffffffff810f20ce>] ? get_empty_filp+0x70/0x12f
	 [<ffffffff810face3>] ? do_filp_open+0x145/0x590
	 [<ffffffff810ce208>] ? tlb_finish_mmu+0x2a/0x33
	 [<ffffffff810ce43c>] ? unmap_region+0xd3/0xe2
	 [<ffffffff810e4393>] ? virt_to_head_page+0x9/0x2d
	 [<ffffffff81103916>] ? alloc_fd+0x69/0x10e
	 [<ffffffff810ef4ed>] ? do_sys_open+0x56/0xfc
	 [<ffffffff81008a02>] ? system_call_fastpath+0x16/0x1b
	Code: 0f 1f 44 00 00 49 89 c6 fa 66 0f 1f 44 00 00 65 4c 8b 04 25 60 e8 00 00 48 8b 45 00 49 01 c0 49 8b 18 48 85 db 74 0d 48 63 45 18 <48> 8b 04 03 49 89 00 eb 14 4c 89 f9 83 ca ff 44 89 e6 48 89 ef
	RIP  [<ffffffff810e61a3>] kmem_cache_alloc+0x5b/0xe9

This problem is that find_keyring_by_name does not confirm that the keyring is
valid before accepting it.

Skipping keyrings that have been reduced to a zero count seems the way to go.
To this end, use atomic_inc_not_zero() to increment the usage count and skip
the candidate keyring if that returns false.

The following script _may_ cause the bug to happen, but there's no guarantee
as the window of opportunity is small:

	#!/bin/sh
	LOOP=100000
	USER=dummy_user
	/bin/su -c "exit;" $USER || { /usr/sbin/adduser -m $USER; add=1; }
	for ((i=0; i<LOOP; i++))
	do
		/bin/su -c "echo '$i' > /dev/null" $USER
	done
	(( add == 1 )) && /usr/sbin/userdel -r $USER
	exit

Note that the nominated user must not be in use.

An alternative way of testing this may be:

	for ((i=0; i<100000; i++))
	do
		keyctl session foo /bin/true || break
	done >&/dev/null

as that uses a keyring named "foo" rather than relying on the user and
user-session named keyrings.

Reported-by: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-05-05 23:49:10 +10:00
David Howells
cf8304e8f3 KEYS: Fix RCU handling in key_gc_keyring()
key_gc_keyring() needs to either hold the RCU read lock or hold the keyring
semaphore if it's going to scan the keyring's list.  Given that it only needs
to read the key list, and it's doing so under a spinlock, the RCU read lock is
the thing to use.

Furthermore, the RCU check added in e7b0a61b79 is
incorrect as holding the spinlock on key_serial_lock is not grounds for
assuming a keyring's pointer list can be read safely.  Instead, a simple
rcu_dereference() inside of the previously mentioned RCU read lock is what we
want.

Reported-by: Serge E. Hallyn <serue@us.ibm.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Acked-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-05-05 11:39:23 +10:00
David Howells
d9a9b4aeea KEYS: Fix an RCU warning in the reading of user keys
Fix an RCU warning in the reading of user keys:

===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
security/keys/user_defined.c:202 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/3637:
 #0:  (&key->sem){+++++.}, at: [<ffffffff811a80ae>] keyctl_read_key+0x9c/0xcf

stack backtrace:
Pid: 3637, comm: keyctl Not tainted 2.6.34-rc5-cachefs #18
Call Trace:
 [<ffffffff81051f6c>] lockdep_rcu_dereference+0xaa/0xb2
 [<ffffffff811aa55f>] user_read+0x47/0x91
 [<ffffffff811a80be>] keyctl_read_key+0xac/0xcf
 [<ffffffff811a8a06>] sys_keyctl+0x75/0xb7
 [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
2010-05-05 11:38:52 +10:00
Linus Torvalds
1600f9def0 Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6:
  keys: don't need to use RCU in keyring_read() as semaphore is held
2010-04-27 16:26:46 -07:00
David Howells
03449cd9ea keys: the request_key() syscall should link an existing key to the dest keyring
The request_key() system call and request_key_and_link() should make a
link from an existing key to the destination keyring (if supplied), not
just from a new key to the destination keyring.

This can be tested by:

	ring=`keyctl newring fred @s`
	keyctl request2 user debug:a a
	keyctl request user debug:a $ring
	keyctl list $ring

If it says:

	keyring is empty

then it didn't work.  If it shows something like:

	1 key in keyring:
	1070462727: --alswrv     0     0 user: debug:a

then it did.

request_key() system call is meant to recursively search all your keyrings for
the key you desire, and, optionally, if it doesn't exist, call out to userspace
to create one for you.

If request_key() finds or creates a key, it should, optionally, create a link
to that key from the destination keyring specified.

Therefore, if, after a successful call to request_key() with a desination
keyring specified, you see the destination keyring empty, the code didn't work
correctly.

If you see the found key in the keyring, then it did - which is what the patch
is required for.

Signed-off-by: David Howells <dhowells@redhat.com>
Cc: James Morris <jmorris@namei.org>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2010-04-27 16:26:03 -07:00