Replace the rcu_assign_pointer() calls with rcu_assign_keypointer().
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
The kernel contains some special internal keyrings, for instance the DNS
resolver keyring :
2a93faf1 I----- 1 perm 1f030000 0 0 keyring .dns_resolver: empty
It would occasionally be useful to allow the contents of such keyrings to be
flushed by root (cache invalidation).
Allow a flag to be set on a keyring to mark that someone possessing the
sysadmin capability can clear the keyring, even without normal write access to
the keyring.
Set this flag on the special keyrings created by the DNS resolver, the NFS
identity mapper and the CIFS identity mapper.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Jeff Layton <jlayton@redhat.com>
Acked-by: Steve Dickson <steved@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
For CIFS, we want to be able to store NTLM credentials (aka username
and password) in the keyring. We do not, however want to allow users
to fetch those keys back out of the keyring since that would be a
security risk.
Unfortunately, due to the nuances of key permission bits, it's not
possible to do this. We need to grant search permissions so the kernel
can find these keys, but that also implies permissions to read the
payload.
Resolve this by adding a new key_type. This key type is essentially
the same as key_type_user, but does not define a .read op. This
prevents the payload from ever being visible from userspace. This
key type also vets the description to ensure that it's "qualified"
by checking to ensure that it has a ':' in it that is preceded by
other characters.
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Enabling CONFIG_PROVE_RCU and CONFIG_SPARSE_RCU_POINTER resulted in
"suspicious rcu_dereference_check() usage!" and "incompatible types
in comparison expression (different address spaces)" messages.
Access the masterkey directly when holding the rwsem.
Changelog v1:
- Use either rcu_read_lock()/rcu_derefence_key()/rcu_read_unlock()
or remove the unnecessary rcu_derefence() - David Howells
Reported-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
Define rcu_assign_keypointer(), which uses the key payload.rcudata instead
of payload.data, to resolve the CONFIG_SPARSE_RCU_POINTER message:
"incompatible types in comparison expression (different address spaces)"
Replace the rcu_assign_pointer() calls in encrypted/trusted keys with
rcu_assign_keypointer().
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
Add missing smp_rmb() primitives to the keyring search code.
When keyring payloads are appended to without replacement (thus using up spare
slots in the key pointer array), an smp_wmb() is issued between the pointer
assignment and the increment of the key count (nkeys).
There should be corresponding read barriers between the read of nkeys and
dereferences of keys[n] when n is dependent on the value of nkeys.
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
Give keys their own lockdep class to differentiate them from each other in case
a key of one type has to refer to a key of another type.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
Encrypted keys are encrypted/decrypted using either a trusted or
user-defined key type, which is referred to as the 'master' key.
The master key may be of type trusted iff the trusted key is
builtin or both the trusted key and encrypted keys are built as
modules. This patch resolves the build dependency problem.
- Use "masterkey-$(CONFIG_TRUSTED_KEYS)-$(CONFIG_ENCRYPTED_KEYS)" construct
to encapsulate the above logic. (Suggested by Dimtry Kasatkin.)
- Fixing the encrypted-keys Makefile, results in a module name change
from encrypted.ko to encrypted-keys.ko.
- Add module dependency for request_trusted_key() definition
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
There is no functional change.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mimi Zohar <zohar@us.ibm.com>
Cc: James Morris <jmorris@namei.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
The basic idea behind cross memory attach is to allow MPI programs doing
intra-node communication to do a single copy of the message rather than a
double copy of the message via shared memory.
The following patch attempts to achieve this by allowing a destination
process, given an address and size from a source process, to copy memory
directly from the source process into its own address space via a system
call. There is also a symmetrical ability to copy from the current
process's address space into a destination process's address space.
- Use of /proc/pid/mem has been considered, but there are issues with
using it:
- Does not allow for specifying iovecs for both src and dest, assuming
preadv or pwritev was implemented either the area read from or
written to would need to be contiguous.
- Currently mem_read allows only processes who are currently
ptrace'ing the target and are still able to ptrace the target to read
from the target. This check could possibly be moved to the open call,
but its not clear exactly what race this restriction is stopping
(reason appears to have been lost)
- Having to send the fd of /proc/self/mem via SCM_RIGHTS on unix
domain socket is a bit ugly from a userspace point of view,
especially when you may have hundreds if not (eventually) thousands
of processes that all need to do this with each other
- Doesn't allow for some future use of the interface we would like to
consider adding in the future (see below)
- Interestingly reading from /proc/pid/mem currently actually
involves two copies! (But this could be fixed pretty easily)
As mentioned previously use of vmsplice instead was considered, but has
problems. Since you need the reader and writer working co-operatively if
the pipe is not drained then you block. Which requires some wrapping to
do non blocking on the send side or polling on the receive. In all to all
communication it requires ordering otherwise you can deadlock. And in the
example of many MPI tasks writing to one MPI task vmsplice serialises the
copying.
There are some cases of MPI collectives where even a single copy interface
does not get us the performance gain we could. For example in an
MPI_Reduce rather than copy the data from the source we would like to
instead use it directly in a mathops (say the reduce is doing a sum) as
this would save us doing a copy. We don't need to keep a copy of the data
from the source. I haven't implemented this, but I think this interface
could in the future do all this through the use of the flags - eg could
specify the math operation and type and the kernel rather than just
copying the data would apply the specified operation between the source
and destination and store it in the destination.
Although we don't have a "second user" of the interface (though I've had
some nibbles from people who may be interested in using it for intra
process messaging which is not MPI). This interface is something which
hardware vendors are already doing for their custom drivers to implement
fast local communication. And so in addition to this being useful for
OpenMPI it would mean the driver maintainers don't have to fix things up
when the mm changes.
There was some discussion about how much faster a true zero copy would
go. Here's a link back to the email with some testing I did on that:
http://marc.info/?l=linux-mm&m=130105930902915&w=2
There is a basic man page for the proposed interface here:
http://ozlabs.org/~cyeoh/cma/process_vm_readv.txt
This has been implemented for x86 and powerpc, other architecture should
mainly (I think) just need to add syscall numbers for the process_vm_readv
and process_vm_writev. There are 32 bit compatibility versions for
64-bit kernels.
For arch maintainers there are some simple tests to be able to quickly
verify that the syscalls are working correctly here:
http://ozlabs.org/~cyeoh/cma/cma-test-20110718.tgz
Signed-off-by: Chris Yeoh <yeohc@au1.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: David Howells <dhowells@redhat.com>
Cc: James Morris <jmorris@namei.org>
Cc: <linux-man@vger.kernel.org>
Cc: <linux-arch@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
For each hex2bin call in encrypted keys, check that the ascii hex string
is valid. On failure, return -EINVAL.
Changelog v1:
- hex2bin now returns an int
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
For each hex2bin call in trusted keys, check that the ascii hex string is
valid. On failure, return -EINVAL.
Changelog v1:
- hex2bin now returns an int
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Fixes this build error:
security/keys/encrypted-keys/masterkey_trusted.c: In function 'request_trusted_key':
security/keys/encrypted-keys/masterkey_trusted.c:35:2: error: implicit declaration of function 'IS_ERR'
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Encrypted keys are decrypted/encrypted using either a trusted-key or,
for those systems without a TPM, a user-defined key. This patch
removes the trusted-keys and TCG_TPM dependencies.
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
unregister_key_type() has code to mark a key as dead and make it unavailable in
one loop and then destroy all those unavailable key payloads in the next loop.
However, the loop to mark keys dead renders the key undetectable to the second
loop by changing the key type pointer also.
Fix this by the following means:
(1) The key code has two garbage collectors: one deletes unreferenced keys and
the other alters keyrings to delete links to old dead, revoked and expired
keys. They can end up holding each other up as both want to scan the key
serial tree under spinlock. Combine these into a single routine.
(2) Move the dead key marking, dead link removal and dead key removal into the
garbage collector as a three phase process running over the three cycles
of the normal garbage collection procedure. This is tracked by the
KEY_GC_REAPING_DEAD_1, _2 and _3 state flags.
unregister_key_type() then just unlinks the key type from the list, wakes
up the garbage collector and waits for the third phase to complete.
(3) Downgrade the key types sem in unregister_key_type() once it has deleted
the key type from the list so that it doesn't block the keyctl() syscall.
(4) Dead keys that cannot be simply removed in the third phase have their
payloads destroyed with the key's semaphore write-locked to prevent
interference by the keyctl() syscall. There should be no in-kernel users
of dead keys of that type by the point of unregistration, though keyctl()
may be holding a reference.
(5) Only perform timer recalculation in the GC if the timer actually expired.
If it didn't, we'll get another cycle when it goes off - and if the key
that actually triggered it has been removed, it's not a problem.
(6) Only garbage collect link if the timer expired or if we're doing dead key
clean up phase 2.
(7) As only key_garbage_collector() is permitted to use rb_erase() on the key
serial tree, it doesn't need to revalidate its cursor after dropping the
spinlock as the node the cursor points to must still exist in the tree.
(8) Drop the spinlock in the GC if there is contention on it or if we need to
reschedule. After dealing with that, get the spinlock again and resume
scanning.
This has been tested in the following ways:
(1) Run the keyutils testsuite against it.
(2) Using the AF_RXRPC and RxKAD modules to test keytype removal:
Load the rxrpc_s key type:
# insmod /tmp/af-rxrpc.ko
# insmod /tmp/rxkad.ko
Create a key (http://people.redhat.com/~dhowells/rxrpc/listen.c):
# /tmp/listen &
[1] 8173
Find the key:
# grep rxrpc_s /proc/keys
091086e1 I--Q-- 1 perm 39390000 0 0 rxrpc_s 52:2
Link it to a session keyring, preferably one with a higher serial number:
# keyctl link 0x20e36251 @s
Kill the process (the key should remain as it's linked to another place):
# fg
/tmp/listen
^C
Remove the key type:
rmmod rxkad
rmmod af-rxrpc
This can be made a more effective test by altering the following part of
the patch:
if (unlikely(gc_state & KEY_GC_REAPING_DEAD_2)) {
/* Make sure everyone revalidates their keys if we marked a
* bunch as being dead and make sure all keyring ex-payloads
* are destroyed.
*/
kdebug("dead sync");
synchronize_rcu();
To call synchronize_rcu() in GC phase 1 instead. That causes that the
keyring's old payload content to hang around longer until it's RCU
destroyed - which usually happens after GC phase 3 is complete. This
allows the destroy_dead_key branch to be tested.
Reported-by: Benjamin Coddington <bcodding@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
The dead key link reaper should be non-reentrant as it relies on global state
to keep track of where it's got to when it returns to the work queue manager to
give it some air.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
Make the key reaper non-reentrant by sticking it on the appropriate system work
queue when we queue it. This will allow it to have global state and drop
locks. It should probably be non-reentrant already as it may spend a long time
holding the key serial spinlock, and so multiple entrants can spend long
periods of time just sitting there spinning, waiting to get the lock.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
Move the unreferenced key reaper function to the keys garbage collector file
as that's a more appropriate place with the dead key link reaper.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
__key_link() should use the RCU deref wrapper rcu_dereference_locked_keyring()
for accessing keyring payloads rather than calling rcu_dereference_protected()
directly.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
The keyctl call:
keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 1)
should create a session keyring if the process doesn't have one of its own
because the create flag argument is set - rather than subscribing to and
returning the user-session keyring as:
keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 0)
will do.
This can be tested by commenting out pam_keyinit in the /etc/pam.d files and
running the following program a couple of times in a row:
#include <stdio.h>
#include <stdlib.h>
#include <keyutils.h>
int main(int argc, char *argv[])
{
key_serial_t uk, usk, sk, nsk;
uk = keyctl_get_keyring_ID(KEY_SPEC_USER_KEYRING, 0);
usk = keyctl_get_keyring_ID(KEY_SPEC_USER_SESSION_KEYRING, 0);
sk = keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 0);
nsk = keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 1);
printf("keys: %08x %08x %08x %08x\n", uk, usk, sk, nsk);
return 0;
}
Without this patch, I see:
keys: 3975ddc7 119c0c66 119c0c66 119c0c66
keys: 3975ddc7 119c0c66 119c0c66 119c0c66
With this patch, I see:
keys: 2cb4997b 34112878 34112878 17db2ce3
keys: 2cb4997b 34112878 34112878 39f3c73e
As can be seen, the session keyring starts off the same as the user-session
keyring each time, but with the patch a new session keyring is created when
the create flag is set.
Reported-by: Greg Wettstein <greg@enjellic.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Greg Wettstein <greg@enjellic.com>
Signed-off-by: James Morris <jmorris@namei.org>
If install_session_keyring() is given a keyring, it should install it rather
than just creating a new one anyway. This was accidentally broken in:
commit d84f4f992c
Author: David Howells <dhowells@redhat.com>
Date: Fri Nov 14 10:39:23 2008 +1100
Subject: CRED: Inaugurate COW credentials
The impact of that commit is that pam_keyinit no longer works correctly if
'force' isn't specified against a login process. This is because:
keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 0)
now always creates a new session keyring and thus the check whether the session
keyring and the user-session keyring are the same is always false. This leads
pam_keyinit to conclude that a session keyring is installed and it shouldn't be
revoked by pam_keyinit here if 'revoke' is specified.
Any system that specifies 'force' against pam_keyinit in the PAM configuration
files for login methods (login, ssh, su -l, kdm, etc.) is not affected since
that bypasses the broken check and forces the creation of a new session keyring
anyway (for which the revoke flag is not cleared) - and any subsequent call to
pam_keyinit really does have a session keyring already installed, and so the
check works correctly there.
Reverting to the previous behaviour will cause the kernel to subscribe the
process to the user-session keyring as its session keyring if it doesn't have a
session keyring of its own. pam_keyinit will detect this and install a new
session keyring anyway (and won't clear the revert flag).
This can be tested by commenting out pam_keyinit in the /etc/pam.d files and
running the following program a couple of times in a row:
#include <stdio.h>
#include <stdlib.h>
#include <keyutils.h>
int main(int argc, char *argv[])
{
key_serial_t uk, usk, sk;
uk = keyctl_get_keyring_ID(KEY_SPEC_USER_KEYRING, 0);
usk = keyctl_get_keyring_ID(KEY_SPEC_USER_SESSION_KEYRING, 0);
sk = keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 0);
printf("keys: %08x %08x %08x\n", uk, usk, sk);
return 0;
}
Without the patch, I see:
keys: 3884e281 24c4dfcf 22825f8e
keys: 3884e281 24c4dfcf 068772be
With the patch, I see:
keys: 26be9c83 0e755ce0 0e755ce0
keys: 26be9c83 0e755ce0 0e755ce0
As can be seen, with the patch, the session keyring is the same as the
user-session keyring each time; without the patch a new session keyring is
generated each time.
Reported-by: Greg Wettstein <greg@enjellic.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Greg Wettstein <greg@enjellic.com>
Signed-off-by: James Morris <jmorris@namei.org>
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6: (54 commits)
tpm_nsc: Fix bug when loading multiple TPM drivers
tpm: Move tpm_tis_reenable_interrupts out of CONFIG_PNP block
tpm: Fix compilation warning when CONFIG_PNP is not defined
TOMOYO: Update kernel-doc.
tpm: Fix a typo
tpm_tis: Probing function for Intel iTPM bug
tpm_tis: Fix the probing for interrupts
tpm_tis: Delay ACPI S3 suspend while the TPM is busy
tpm_tis: Re-enable interrupts upon (S3) resume
tpm: Fix display of data in pubek sysfs entry
tpm_tis: Add timeouts sysfs entry
tpm: Adjust interface timeouts if they are too small
tpm: Use interface timeouts returned from the TPM
tpm_tis: Introduce durations sysfs entry
tpm: Adjust the durations if they are too small
tpm: Use durations returned from TPM
TOMOYO: Enable conditional ACL.
TOMOYO: Allow using argv[]/envp[] of execve() as conditions.
TOMOYO: Allow using executable's realpath and symlink's target as conditions.
TOMOYO: Allow using owner/group etc. of file objects as conditions.
...
Fix up trivial conflict in security/tomoyo/realpath.c
Since ca5ecddf (rcu: define __rcu address space modifier for sparse)
rcu_dereference_check use rcu_read_lock_held as a part of condition
automatically so callers do not have to do that as well.
Signed-off-by: Michal Hocko <mhocko@suse.cz>
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
The 'encrypted' key type defines its own payload format which contains a
symmetric key randomly generated that cannot be used directly to mount
an eCryptfs filesystem, because it expects an authentication token
structure.
This patch introduces the new format 'ecryptfs' that allows to store an
authentication token structure inside the encrypted key payload containing
a randomly generated symmetric key, as the same for the format 'default'.
More details about the usage of encrypted keys with the eCryptfs
filesystem can be found in the file 'Documentation/keys-ecryptfs.txt'.
Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Acked-by: Gianluca Ramunno <ramunno@polito.it>
Acked-by: Tyler Hicks <tyhicks@linux.vnet.ibm.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
This patch introduces a new parameter, called 'format', that defines the
format of data stored by encrypted keys. The 'default' format identifies
encrypted keys containing only the symmetric key, while other formats can
be defined to support additional information. The 'format' parameter is
written in the datablob produced by commands 'keyctl print' or
'keyctl pipe' and is integrity protected by the HMAC.
Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Acked-by: Gianluca Ramunno <ramunno@polito.it>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Some debug messages have been added in the function datablob_parse() in
order to better identify errors returned when dealing with 'encrypted'
keys.
Changelog from version v4:
- made the debug messages more understandable
Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Acked-by: Gianluca Ramunno <ramunno@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Do not dump the master key if an error is encountered during the request.
Signed-off-by: Roberto Sassu <roberto.sassu@polito.it>
Acked-by: Gianluca Ramunno <ramunno@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Fix error handling in construct_key_and_link().
If construct_alloc_key() returns an error, it shouldn't pass out through
the normal path as the key_serial() called by the kleave() statement
will oops when it gets an error code in the pointer:
BUG: unable to handle kernel paging request at ffffffffffffff84
IP: [<ffffffff8120b401>] request_key_and_link+0x4d7/0x52f
..
Call Trace:
[<ffffffff8120b52c>] request_key+0x41/0x75
[<ffffffffa00ed6e8>] cifs_get_spnego_key+0x206/0x226 [cifs]
[<ffffffffa00eb0c9>] CIFS_SessSetup+0x511/0x1234 [cifs]
[<ffffffffa00d9799>] cifs_setup_session+0x90/0x1ae [cifs]
[<ffffffffa00d9c02>] cifs_get_smb_ses+0x34b/0x40f [cifs]
[<ffffffffa00d9e05>] cifs_mount+0x13f/0x504 [cifs]
[<ffffffffa00caabb>] cifs_do_mount+0xc4/0x672 [cifs]
[<ffffffff8113ae8c>] mount_fs+0x69/0x155
[<ffffffff8114ff0e>] vfs_kern_mount+0x63/0xa0
[<ffffffff81150be2>] do_kern_mount+0x4d/0xdf
[<ffffffff81152278>] do_mount+0x63c/0x69f
[<ffffffff8115255c>] sys_mount+0x88/0xc2
[<ffffffff814fbdc2>] system_call_fastpath+0x16/0x1b
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
____call_usermodehelper() now erases any credentials set by the
subprocess_inf::init() function. The problem is that commit
17f60a7da1 ("capabilites: allow the application of capability limits
to usermode helpers") creates and commits new credentials with
prepare_kernel_cred() after the call to the init() function. This wipes
all keyrings after umh_keys_init() is called.
The best way to deal with this is to put the init() call just prior to
the commit_creds() call, and pass the cred pointer to init(). That
means that umh_keys_init() and suchlike can modify the credentials
_before_ they are published and potentially in use by the rest of the
system.
This prevents request_key() from working as it is prevented from passing
the session keyring it set up with the authorisation token to
/sbin/request-key, and so the latter can't assume the authority to
instantiate the key. This causes the in-kernel DNS resolver to fail
with ENOKEY unconditionally.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Eric Paris <eparis@redhat.com>
Tested-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Don't return EAGAIN to keyctl_assume_authority() to indicate that a key could
not be found (ENOKEY is only returned if a negative key is found). Instead
return ENOKEY in both cases.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
* 'docs-move' of git://git.kernel.org/pub/scm/linux/kernel/git/rdunlap/linux-docs:
Create Documentation/security/, move LSM-, credentials-, and keys-related files from Documentation/ to Documentation/security/, add Documentation/security/00-INDEX, and update all occurrences of Documentation/<moved_file> to Documentation/security/<moved_file>.
Since this cred was not created with copy_creds(), it needs to get
initialized. Otherwise use of syscall(__NR_keyctl, KEYCTL_SESSION_TO_PARENT);
can lead to a NULL deref. Thanks to Robert for finding this.
But introduced by commit 47a150edc2 ("Cache user_ns in struct cred").
Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com>
Reported-by: Robert Święcki <robert@swiecki.net>
Cc: David Howells <dhowells@redhat.com>
Cc: stable@kernel.org (2.6.39)
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
move LSM-, credentials-, and keys-related files from Documentation/
to Documentation/security/,
add Documentation/security/00-INDEX, and
update all occurrences of Documentation/<moved_file>
to Documentation/security/<moved_file>.
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>
Make request_key() and co. return an error for a negative or rejected key. If
the key was simply negated, then return ENOKEY, otherwise return the error
with which it was rejected.
Without this patch, the following command returns a key number (with the latest
keyutils):
[root@andromeda ~]# keyctl request2 user debug:foo rejected @s
586569904
Trying to print the key merely gets you a permission denied error:
[root@andromeda ~]# keyctl print 586569904
keyctl_read_alloc: Permission denied
Doing another request_key() call does get you the error, as long as it hasn't
expired yet:
[root@andromeda ~]# keyctl request user debug:foo
request_key: Key was rejected by service
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
Improve /proc/keys by:
(1) Don't attempt to summarise the payload of a negated key. It won't have
one. To this end, a helper function - key_is_instantiated() has been
added that allows the caller to find out whether the key is positively
instantiated (as opposed to being uninstantiated or negatively
instantiated).
(2) Do show keys that are negative, expired or revoked rather than hiding
them. This requires an override flag (no_state_check) to be passed to
search_my_process_keyrings() and keyring_search_aux() to suppress this
check.
Without this, keys that are possessed by the caller, but only grant
permissions to the caller if possessed are skipped as the possession check
fails.
Keys that are visible due to user, group or other checks are visible with
or without this patch.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
- 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>
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>
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>
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>
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>
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>
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>
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>
* '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
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>
keyring_read() doesn't need to use rcu_dereference() to access the keyring
payload as the caller holds the key semaphore to prevent modifications
from happening whilst the data is read out.
This should solve the following warning:
===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
security/keys/keyring.c:204 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/2144:
#0: (&key->sem){+++++.}, at: [<ffffffff81177f7c>] keyctl_read_key+0x9c/0xcf
stack backtrace:
Pid: 2144, comm: keyctl Not tainted 2.6.34-rc2-cachefs #113
Call Trace:
[<ffffffff8105121f>] lockdep_rcu_dereference+0xaa/0xb2
[<ffffffff811762d5>] keyring_read+0x4d/0xe7
[<ffffffff81177f8c>] keyctl_read_key+0xac/0xcf
[<ffffffff811788d4>] sys_keyctl+0x75/0xb9
[<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b
Signed-off-by: David Howells <dhowells@redhat.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: James Morris <jmorris@namei.org>
Fix the following RCU warning:
===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
security/keys/request_key.c:116 invoked rcu_dereference_check() without protection!
This was caused by doing:
[root@andromeda ~]# keyctl newring fred @s
539196288
[root@andromeda ~]# keyctl request2 user a a 539196288
request_key: Required key not available
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
percpu.h is included by sched.h and module.h and thus ends up being
included when building most .c files. percpu.h includes slab.h which
in turn includes gfp.h making everything defined by the two files
universally available and complicating inclusion dependencies.
percpu.h -> slab.h dependency is about to be removed. Prepare for
this change by updating users of gfp and slab facilities include those
headers directly instead of assuming availability. As this conversion
needs to touch large number of source files, the following script is
used as the basis of conversion.
http://userweb.kernel.org/~tj/misc/slabh-sweep.py
The script does the followings.
* Scan files for gfp and slab usages and update includes such that
only the necessary includes are there. ie. if only gfp is used,
gfp.h, if slab is used, slab.h.
* When the script inserts a new include, it looks at the include
blocks and try to put the new include such that its order conforms
to its surrounding. It's put in the include block which contains
core kernel includes, in the same order that the rest are ordered -
alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
doesn't seem to be any matching order.
* If the script can't find a place to put a new include (mostly
because the file doesn't have fitting include block), it prints out
an error message indicating which .h file needs to be added to the
file.
The conversion was done in the following steps.
1. The initial automatic conversion of all .c files updated slightly
over 4000 files, deleting around 700 includes and adding ~480 gfp.h
and ~3000 slab.h inclusions. The script emitted errors for ~400
files.
2. Each error was manually checked. Some didn't need the inclusion,
some needed manual addition while adding it to implementation .h or
embedding .c file was more appropriate for others. This step added
inclusions to around 150 files.
3. The script was run again and the output was compared to the edits
from #2 to make sure no file was left behind.
4. Several build tests were done and a couple of problems were fixed.
e.g. lib/decompress_*.c used malloc/free() wrappers around slab
APIs requiring slab.h to be added manually.
5. The script was run on all .h files but without automatically
editing them as sprinkling gfp.h and slab.h inclusions around .h
files could easily lead to inclusion dependency hell. Most gfp.h
inclusion directives were ignored as stuff from gfp.h was usually
wildly available and often used in preprocessor macros. Each
slab.h inclusion directive was examined and added manually as
necessary.
6. percpu.h was updated not to include slab.h.
7. Build test were done on the following configurations and failures
were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
distributed build env didn't work with gcov compiles) and a few
more options had to be turned off depending on archs to make things
build (like ipr on powerpc/64 which failed due to missing writeq).
* x86 and x86_64 UP and SMP allmodconfig and a custom test config.
* powerpc and powerpc64 SMP allmodconfig
* sparc and sparc64 SMP allmodconfig
* ia64 SMP allmodconfig
* s390 SMP allmodconfig
* alpha SMP allmodconfig
* um on x86_64 SMP allmodconfig
8. percpu.h modifications were reverted so that it could be applied as
a separate patch and serve as bisection point.
Given the fact that I had only a couple of failures from tests on step
6, I'm fairly confident about the coverage of this conversion patch.
If there is a breakage, it's likely to be something in one of the arch
headers which should be easily discoverable easily on most builds of
the specific arch.
Signed-off-by: Tejun Heo <tj@kernel.org>
Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
This fixes to include <linux/uaccess.h> instead <asm/uaccess.h> and some
code style issues like to put a else sentence below close brace '}' and
to replace a tab instead of some space characters.
Signed-off-by: Chihau Chau <chihau@gmail.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
Fix some coding styles in security/keys/keyring.c
Signed-off-by: Zhitong Wang <zhitong.wangzt@alibaba-inc.com>
Signed-off-by: James Morris <jmorris@namei.org>
