When commit be6d3e56a6 "introduce new LSM hooks
where vfsmount is available." was proposed, regarding security_path_truncate(),
only "struct file *" argument (which AppArmor wanted to use) was removed.
But length and time_attrs arguments are not used by TOMOYO nor AppArmor.
Thus, let's remove these arguments.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Acked-by: Nick Piggin <npiggin@suse.de>
Signed-off-by: James Morris <jmorris@namei.org>
This patch removes some unneeded code for if opt_dentry is null because
that can never happen.
The function dereferences "opt_dentry" earlier when it checks
"if (opt_dentry->d_parent == opt_dentry) {". That code was added in
2008.
This function called from security_d_instantiate(). I checked all the
places which call security_d_instantiate() and dentry is always non-null.
I also checked the selinux version of this hook and there is a comment
which says that dentry should be non-null if called from
d_instantiate().
Signed-off-by: Dan Carpenter <error27@gmail.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: James Morris <jmorris@namei.org>
security/tomoyo/common.c became too large to read.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <jmorris@namei.org>
Since the behavior of the system is restricted by policy, we may need to update
policy when you update packages.
We need to update policy in the following cases.
* The pathname of files has changed.
* The dependency of files has changed.
* The access permissions required has increased.
The ideal way to update policy is to rebuild from the scratch using learning
mode. But it is not desirable to change from enforcing mode to other mode if
the system has once entered in production state. Suppose MAC could support
per-application enforcing mode, the MAC becomes useless if an application that
is not running in enforcing mode was cracked. For example, the whole system
becomes vulnerable if only HTTP server application is running in learning mode
to rebuild policy for the application. So, in TOMOYO Linux, updating policy is
done while the system is running in enforcing mode.
This patch implements "interactive enforcing mode" which allows administrators
to judge whether to accept policy violation in enforcing mode or not.
A demo movie is available at http://www.youtube.com/watch?v=b9q1Jo25LPA .
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <jmorris@namei.org>
mount(2) has three string and one numeric parameters.
Split mount restriction code from security/tomoyo/file.c .
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <jmorris@namei.org>
Check numeric parameters for operations that deal them
(e.g. chmod/chown/ioctl).
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <jmorris@namei.org>
This patch adds numeric values grouping support, which is useful for grouping
numeric values such as file's UID, DAC's mode, ioctl()'s cmd number.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <jmorris@namei.org>
Remove all rcu head inits. We don't care about the RCU head state before passing
it to call_rcu() anyway. Only leave the "on_stack" variants so debugobjects can
keep track of objects on stack.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Reviewed-by: James Morris <jmorris@namei.org>
Cc: Eric Paris <eparis@parisplace.org>
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>
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 <neilb@suse.de>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
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>
register_security() became __init function.
So do verify() and security_fixup_ops().
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <jmorris@namei.org>
This patch adds pathname grouping support, which is useful for grouping
pathnames that cannot be represented using /\{dir\}/ pattern.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <jmorris@namei.org>
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 <zohar@us.ibm.com>
Reported-by: Jean-Christophe Dubois <jcd@tribudubois.net>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Tested-by: Serge Hallyn <serue@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
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/)
// <smpl>
@@
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);
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
Acked-by: Eric Paris <eparis@redhat.com>
Signed-off-by: James Morris <jmorris@namei.org>
Use stack memory for pending entry to reduce kmalloc() which will be kfree()d.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
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>
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 <penguin-kernel@I-love.SAKURA.ne.jp>
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>
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 <penguin-kernel@I-love.SAKURA.ne.jp>
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>
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 <zohar@us.ibm.com>
Reported-by: Jean-Christophe Dubois <jcd@tribudubois.net>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Tested-by: Serge Hallyn <serue@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
On Tue, 2010-04-27 at 11:47 -0700, David Miller wrote:
> From: "Tom \"spot\" Callaway" <tcallawa@redhat.com>
> 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 <sds@tycho.nsa.gov>
Acked-by: David S. Miller <davem@davemloft.net>
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>
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 <eparis@redhat.com>
Signed-off-by: James Morris <jmorris@namei.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>
Redirecting directly to lsm, here's the patch discussed on lkml:
http://lkml.org/lkml/2010/4/22/219
The 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 <kees.cook@canonical.com>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
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 <eparis@redhat.com>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
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 <error27@gmail.com>
Signed-off-by: James Morris <jmorris@namei.org>
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 <eparis@redhat.com>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
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 <eparis@redhat.com>
Acked-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
