27 Jun, 2010
1 commit
-
This is from a Smatch check I'm writing.
strncpy_from_user() returns -EFAULT on error so the first change just
silences a warning but doesn't change how the code works.The other change is a bug fix because install_thread_keyring_to_cred()
can return a variety of errors such as -EINVAL, -EEXIST, -ENOMEM or
-EKEYREVOKED.Signed-off-by: Dan Carpenter
Signed-off-by: David Howells
Signed-off-by: Linus Torvalds
28 May, 2010
2 commits
-
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
Acked-by: David Howells
Cc: Peter Zijlstra
Acked-by: Roland McGrath
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds -
call_usermodehelper_keys() uses call_usermodehelper_setkeys() to change
subprocess_info->cred in advance. Now that we have info->init() we can
change this code to set tgcred->session_keyring in context of execing
kernel thread.Note: since currently call_usermodehelper_keys() is never called with
UMH_NO_WAIT, call_usermodehelper_keys()->key_get() and umh_keys_cleanup()
are not really needed, we could rely on install_session_keyring_to_cred()
which does key_get() on success.Signed-off-by: Oleg Nesterov
Acked-by: Neil Horman
Acked-by: David Howells
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
25 May, 2010
1 commit
-
- 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
Acked-by: WANG Cong
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
22 May, 2010
2 commits
-
... kill their private list, while we are at it
Signed-off-by: Al Viro
-
Of the three uses of kref_set in the kernel:
One really should be kref_put as the code is letting go of a
reference,
Two really should be kref_init because the kref is being
initialised.This suggests that making kref_set available encourages bad code.
So fix the three uses and remove kref_set completely.Signed-off-by: NeilBrown
Acked-by: Mimi Zohar
Acked-by: Serge Hallyn
Signed-off-by: Greg Kroah-Hartman
18 May, 2010
1 commit
-
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
Signed-off-by: David Howells
Signed-off-by: James Morris
17 May, 2010
4 commits
-
register_security() became __init function.
So do verify() and security_fixup_ops().Signed-off-by: Tetsuo Handa
Signed-off-by: James Morris -
This patch adds pathname grouping support, which is useful for grouping
pathnames that cannot be represented using /\{dir\}/ pattern.Signed-off-by: Tetsuo Handa
Signed-off-by: James Morris -
The ACPI dependency moved to the TPM, where it belongs. Although
IMA per-se does not require access to the bios measurement log,
verifying the IMA boot aggregate does, which requires ACPI.This patch prereq's 'TPM: ACPI/PNP dependency removal'
http://lkml.org/lkml/2010/5/4/378.Signed-off-by: Mimi Zohar
Reported-by: Jean-Christophe Dubois
Acked-by: Serge Hallyn
Tested-by: Serge Hallyn
Signed-off-by: James Morris -
Use kstrdup when the goal of an allocation is copy a string into the
allocated region.The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)//
@@
expression from,to;
expression flag,E1,E2;
statement S;
@@- to = kmalloc(strlen(from) + 1,flag);
+ to = kstrdup(from, flag);
... when != \(from = E1 \| to = E1 \)
if (to==NULL || ...) S
... when != \(from = E2 \| to = E2 \)
- strcpy(to, from);
//Signed-off-by: Julia Lawall
Acked-by: Eric Paris
Signed-off-by: James Morris
10 May, 2010
1 commit
-
Use stack memory for pending entry to reduce kmalloc() which will be kfree()d.
Signed-off-by: Tetsuo Handa
Signed-off-by: James Morris
07 May, 2010
1 commit
-
This reverts commit a674fa46c79ffa37995bd1c8e4daa2b3be5a95ae.
Previous revert was a prereq.
Signed-off-by: James Morris
06 May, 2010
6 commits
-
Do preallocation for __key_link() so that the various callers in request_key.c
can deal with any errors from this source before attempting to construct a key.
This allows them to assume that the actual linkage step is guaranteed to be
successful.Signed-off-by: David Howells
Signed-off-by: James Morris -
Conflicts:
security/keys/keyring.cResolved conflict with whitespace fix in find_keyring_by_name()
Signed-off-by: James Morris
-
Some of TOMOYO's functions may sleep after mutex_lock(). If OOM-killer selected
a process which is waiting at mutex_lock(), the to-be-killed process can't be
killed. Thus, replace mutex_lock() with mutex_lock_interruptible() so that the
to-be-killed process can immediately return from TOMOYO's functions.Signed-off-by: Tetsuo Handa
Signed-off-by: James Morris -
Errors from construct_alloc_key() shouldn't just be ignored in the way they are
by construct_key_and_link(). The only error that can be ignored so is
EINPROGRESS as that is used to indicate that we've found a key and don't need
to construct one.We don't, however, handle ENOMEM, EDQUOT or EACCES to indicate allocation
failures of one sort or another.Reported-by: Vegard Nossum
Signed-off-by: David Howells
Signed-off-by: James Morris -
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
Signed-off-by: James Morris
05 May, 2010
7 commits
-
In Ubuntu, security_path_*() hooks are exported to Unionfs. Thus, prepare for
being called from inside VFS functions because I'm not sure whether it is safe
to use GFP_KERNEL or not.Signed-off-by: Tetsuo Handa
Signed-off-by: James Morris -
call_sbin_request_key() creates a keyring and then attempts to insert a link to
the authorisation key into that keyring, but does so without holding a write
lock on the keyring semaphore.It will normally get away with this because it hasn't told anyone that the
keyring exists yet. The new keyring, however, has had its serial number
published, which means it can be accessed directly by that handle.This was found by a previous patch that adds RCU lockdep checks to the code
that reads the keyring payload pointer, which includes a check that the keyring
semaphore is actually locked.Without this patch, the following command:
keyctl request2 user b a @s
will provoke the following lockdep warning is displayed in dmesg:
===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
security/keys/keyring.c:727 invoked rcu_dereference_check() without protection!other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 0
2 locks held by keyctl/2076:
#0: (key_types_sem){.+.+.+}, at: [] key_type_lookup+0x1c/0x71
#1: (keyring_serialise_link_sem){+.+.+.}, at: [] __key_link+0x4d/0x3c5stack backtrace:
Pid: 2076, comm: keyctl Not tainted 2.6.34-rc6-cachefs #54
Call Trace:
[] lockdep_rcu_dereference+0xaa/0xb2
[] ? __key_link+0x4d/0x3c5
[] __key_link+0x19e/0x3c5
[] ? __key_instantiate_and_link+0xb1/0xdc
[] ? key_instantiate_and_link+0x42/0x5f
[] call_sbin_request_key+0xe7/0x33b
[] ? mutex_unlock+0x9/0xb
[] ? __key_instantiate_and_link+0xb1/0xdc
[] ? key_instantiate_and_link+0x42/0x5f
[] ? request_key_auth_new+0x1c2/0x23c
[] ? cache_alloc_debugcheck_after+0x108/0x173
[] ? request_key_and_link+0x146/0x300
[] ? kmem_cache_alloc+0xe1/0x118
[] request_key_and_link+0x28b/0x300
[] sys_request_key+0xf7/0x14a
[] ? trace_hardirqs_on_caller+0x10c/0x130
[] ? trace_hardirqs_on_thunk+0x3a/0x3f
[] system_call_fastpath+0x16/0x1bSigned-off-by: David Howells
Signed-off-by: James Morris -
The keyring key type code should use RCU dereference wrappers, even when it
holds the keyring's key semaphore.Reported-by: Vegard Nossum
Signed-off-by: David Howells
Acked-by: Serge Hallyn
Signed-off-by: James Morris -
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() [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 ***
TIMEIf 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=0xffff880197a7e300Bytes 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 jkkkkkkkkkkkkkkkAlternatively, we may see a system panic happen, such as:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000001
IP: [] 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:[] [] 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:
[] ? get_empty_filp+0x70/0x12f
[] ? do_filp_open+0x145/0x590
[] ? tlb_finish_mmu+0x2a/0x33
[] ? unmap_region+0xd3/0xe2
[] ? virt_to_head_page+0x9/0x2d
[] ? alloc_fd+0x69/0x10e
[] ? do_sys_open+0x56/0xfc
[] ? 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 8b 04 03 49 89 00 eb 14 4c 89 f9 83 ca ff 44 89 e6 48 89 ef
RIP [] kmem_cache_alloc+0x5b/0xe9This 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&/dev/nullas that uses a keyring named "foo" rather than relying on the user and
user-session named keyrings.Reported-by: Toshiyuki Okajima
Signed-off-by: David Howells
Tested-by: Toshiyuki Okajima
Acked-by: Serge Hallyn
Signed-off-by: James Morris -
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 e7b0a61b7929632d36cf052d9e2820ef0a9c1bfe 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
Signed-off-by: David Howells
Acked-by: Serge Hallyn
Acked-by: "Paul E. McKenney"
Signed-off-by: James Morris -
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: [] keyctl_read_key+0x9c/0xcfstack backtrace:
Pid: 3637, comm: keyctl Not tainted 2.6.34-rc5-cachefs #18
Call Trace:
[] lockdep_rcu_dereference+0xaa/0xb2
[] user_read+0x47/0x91
[] keyctl_read_key+0xac/0xcf
[] sys_keyctl+0x75/0xb7
[] system_call_fastpath+0x16/0x1bSigned-off-by: David Howells
Acked-by: Serge Hallyn
Signed-off-by: James Morris -
The ACPI dependency moved to the TPM, where it belongs. Although
IMA per-se does not require access to the bios measurement log,
verifying the IMA boot aggregate does, which requires ACPI.This patch prereq's 'TPM: ACPI/PNP dependency removal'
http://lkml.org/lkml/2010/5/4/378.Signed-off-by: Mimi Zohar
Reported-by: Jean-Christophe Dubois
Acked-by: Serge Hallyn
Tested-by: Serge Hallyn
Signed-off-by: James Morris
29 Apr, 2010
1 commit
-
On Tue, 2010-04-27 at 11:47 -0700, David Miller wrote:
> From: "Tom \"spot\" Callaway"
> Date: Tue, 27 Apr 2010 14:20:21 -0400
>
> > [root@apollo ~]$ cat /proc/2174/maps
> > 00010000-00014000 r-xp 00000000 fd:00 15466577
> > /sbin/mingetty
> > 00022000-00024000 rwxp 00002000 fd:00 15466577
> > /sbin/mingetty
> > 00024000-00046000 rwxp 00000000 00:00 0
> > [heap]
>
> SELINUX probably barfs on the executable heap, the PLT is in the HEAP
> just like powerpc32 and that's why VM_DATA_DEFAULT_FLAGS has to set
> both executable and writable.
>
> You also can't remove the CONFIG_PPC32 ifdefs in selinux, since
> because of the VM_DATA_DEFAULT_FLAGS setting used still in that arch,
> the heap will always have executable permission, just like sparc does.
> You have to support those binaries forever, whether you like it or not.
>
> Let's just replace the CONFIG_PPC32 ifdef in SELINUX with CONFIG_PPC32
> || CONFIG_SPARC as in Tom's original patch and let's be done with
> this.
>
> In fact I would go through all the arch/ header files and check the
> VM_DATA_DEFAULT_FLAGS settings and add the necessary new ifdefs to the
> SELINUX code so that other platforms don't have the pain of having to
> go through this process too.To avoid maintaining per-arch ifdefs, it seems that we could just
directly use (VM_DATA_DEFAULT_FLAGS & VM_EXEC) as the basis for deciding
whether to enable or disable these checks. VM_DATA_DEFAULT_FLAGS isn't
constant on some architectures but instead depends on
current->personality, but we want this applied uniformly. So we'll just
use the initial task state to determine whether or not to enable these
checks.Signed-off-by: Stephen Smalley
Acked-by: David S. Miller
Signed-off-by: James Morris
28 Apr, 2010
4 commits
-
…s/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 -
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 $ringIf 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:athen 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
Cc: James Morris
Cc:
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds -
Most of the LSM common audit work uses LSM_AUDIT_DATA_* for the naming.
This was not so for LSM_AUDIT_NO_AUDIT which means the generic initializer
cannot be used. This patch just renames the flag so the generic
initializer can be used.Signed-off-by: Eric Paris
Signed-off-by: James Morris -
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: [] keyctl_read_key+0x9c/0xcfstack backtrace:
Pid: 2144, comm: keyctl Not tainted 2.6.34-rc2-cachefs #113
Call Trace:
[] lockdep_rcu_dereference+0xaa/0xb2
[] keyring_read+0x4d/0xe7
[] keyctl_read_key+0xac/0xcf
[] sys_keyctl+0x75/0xb9
[] system_call_fastpath+0x16/0x1bSigned-off-by: David Howells
Cc: Herbert Xu
Signed-off-by: Andrew Morton
Signed-off-by: James Morris
27 Apr, 2010
1 commit
-
Don't #include Ext2 headers into Smack unnecessarily.
Signed-off-by: David Howells
Acked-by: Casey Schaufler
Signed-off-by: James Morris
25 Apr, 2010
1 commit
-
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 availableSigned-off-by: David Howells
Acked-by: Eric Dumazet
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
23 Apr, 2010
3 commits
-
Whitespace coding style fixes.
Signed-off-by: Justin P. Mattock
Signed-off-by: James Morris -
Redirecting directly to lsm, here's the patch discussed on lkml:
http://lkml.org/lkml/2010/4/22/219The mmap_min_addr value is useful information for an admin to see without
being root ("is my system vulnerable to kernel NULL pointer attacks?") and
its setting is trivially easy for an attacker to determine by calling
mmap() in PAGE_SIZE increments starting at 0, so trying to keep it private
has no value.Only require CAP_SYS_RAWIO if changing the value, not reading it.
Comment from Serge :
Me, I like to write my passwords with light blue pen on dark blue
paper, pasted on my window - if you're going to get my password, you're
gonna get a headache.Signed-off-by: Kees Cook
Acked-by: Serge Hallyn
Signed-off-by: James Morris -
As an example IMA emits a warning when it can't find a TPM chip:
"No TPM chip found, activating TPM-bypass!"
This patch prefaces that message with IMA so we know what subsystem is
bypassing the TPM. Do this for all pr_info and pr_err messages.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris
22 Apr, 2010
1 commit
-
There is a typo here. We should be testing "*dentry" instead of
"dentry". If "*dentry" is an ERR_PTR, it gets dereferenced in either
mkdir() or create() which would cause an OOPs.Signed-off-by: Dan Carpenter
Signed-off-by: James Morris
21 Apr, 2010
3 commits
-
integrity_audit_msg() uses "integrity:" in the audit message. This
violates the (loosely defined) audit system requirements that everything be
a key=value pair and it doesn't provide additional information. This can
be obviously gleaned from the message type. Just drop it.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris -
Convert all of the places IMA calls audit_log_format with %s into
audit_log_untrusted_string(). This is going to cause them all to get
quoted, but it should make audit log injection harder.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris -
IMA policy load parser will reject any policies with a comment. This patch
will allow the parser to just ignore lines which start with a #. This is not
very robust. # can ONLY be used at the very beginning of a line. Inline
comments are not allowed.Signed-off-by: Eric Paris
Acked-by: Mimi Zohar
Signed-off-by: James Morris