selinux_inode_has_perm is a hot path. Instead of declaring the
common_audit_data on the stack move it to a noinline function only used in
the rare case we need to send an audit message.
Signed-off-by: Eric Paris <eparis@redhat.com>
We pay a rather large overhead initializing the common_audit_data.
Since we only need this information if we actually emit an audit
message there is little need to set it up in the hot path. This patch
splits the functionality of avc_has_perm() into avc_has_perm_noaudit(),
avc_audit_required() and slow_avc_audit(). But we take care of setting
up to audit between required() and the actual audit call. Thus saving
measurable time in a hot path.
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: Eric Paris <eparis@redhat.com>
We reset the bool names and values array to NULL, but do not reset the
number of entries in these arrays to 0. If we error out and then get back
into this function we will walk these NULL pointers based on the belief
that they are non-zero length.
Signed-off-by: Eric Paris <eparis@redhat.com>
cc: stable@kernel.org
I'm not really sure what the idea behind the sel_div function is, but it's
useless. Since a and b are both unsigned, it's impossible for a % b < 0.
That means that part of the function never does anything. Thus it's just a
normal /. Just do that instead. I don't even understand what that operation
was supposed to mean in the signed case however....
If it was signed:
sel_div(-2, 4) == ((-2 / 4) - ((-2 % 4) < 0))
((0) - ((-2) < 0))
((0) - (1))
(-1)
What actually happens:
sel_div(-2, 4) == ((18446744073709551614 / 4) - ((18446744073709551614 % 4) < 0))
((4611686018427387903) - ((2 < 0))
(4611686018427387903 - 0)
((unsigned int)4611686018427387903)
(4294967295)
Neither makes a whole ton of sense to me. So I'm getting rid of the
function entirely.
Signed-off-by: Eric Paris <eparis@redhat.com>
It's possible that the caller passed a NULL for scontext. However if this
is a defered mapping we might still attempt to call *scontext=kstrdup().
This is bad. Instead just return the len.
Signed-off-by: Eric Paris <eparis@redhat.com>
We know that some yum operation is causing CAP_MAC_ADMIN failures. This
implies that an RPM is laying down (or attempting to lay down) a file with
an invalid label. The problem is that we don't have any information to
track down the cause. This patch will cause such a failure to report the
failed label in an SELINUX_ERR audit message. This is similar to the
SELINUX_ERR reports on invalid transitions and things like that. It should
help run down problems on what is trying to set invalid labels in the
future.
Resulting records look something like:
type=AVC msg=audit(1319659241.138:71): avc: denied { mac_admin } for pid=2594 comm="chcon" capability=33 scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=capability2
type=SELINUX_ERR msg=audit(1319659241.138:71): op=setxattr invalid_context=unconfined_u:object_r:hello:s0
type=SYSCALL msg=audit(1319659241.138:71): arch=c000003e syscall=188 success=no exit=-22 a0=a2c0e0 a1=390341b79b a2=a2d620 a3=1f items=1 ppid=2519 pid=2594 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=1 comm="chcon" exe="/usr/bin/chcon" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
type=CWD msg=audit(1319659241.138:71): cwd="/root" type=PATH msg=audit(1319659241.138:71): item=0 name="test" inode=785879 dev=fc:03 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:admin_home_t:s0
Signed-off-by: Eric Paris <eparis@redhat.com>
In RH BZ 578841 we realized that the SELinux sandbox program was allowed to
truncate files outside of the sandbox. The reason is because sandbox
confinement is determined almost entirely by the 'open' permission. The idea
was that if the sandbox was unable to open() files it would be unable to do
harm to those files. This turns out to be false in light of syscalls like
truncate() and chmod() which don't require a previous open() call. I looked
at the syscalls that did not have an associated 'open' check and found that
truncate(), did not have a seperate permission and even if it did have a
separate permission such a permission owuld be inadequate for use by
sandbox (since it owuld have to be granted so liberally as to be useless).
This patch checks the OPEN permission on truncate. I think a better solution
for sandbox is a whole new permission, but at least this fixes what we have
today.
Signed-off-by: Eric Paris <eparis@redhat.com>
Because Fedora shipped userspace based on my development tree we now
have policy version 27 in the wild defining only default user, role, and
range. Thus to add default_type we need a policy.28.
Signed-off-by: Eric Paris <eparis@redhat.com>
When new objects are created we have great and flexible rules to
determine the type of the new object. We aren't quite as flexible or
mature when it comes to determining the user, role, and range. This
patch adds a new ability to specify the place a new objects user, role,
and range should come from. For users and roles it can come from either
the source or the target of the operation. aka for files the user can
either come from the source (the running process and todays default) or
it can come from the target (aka the parent directory of the new file)
examples always are done with
directory context: system_u:object_r:mnt_t:s0-s0:c0.c512
process context: unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
[no rule]
unconfined_u:object_r:mnt_t:s0 test_none
[default user source]
unconfined_u:object_r:mnt_t:s0 test_user_source
[default user target]
system_u:object_r:mnt_t:s0 test_user_target
[default role source]
unconfined_u:unconfined_r:mnt_t:s0 test_role_source
[default role target]
unconfined_u:object_r:mnt_t:s0 test_role_target
[default range source low]
unconfined_u:object_r:mnt_t:s0 test_range_source_low
[default range source high]
unconfined_u:object_r:mnt_t:s0:c0.c1023 test_range_source_high
[default range source low-high]
unconfined_u:object_r:mnt_t:s0-s0:c0.c1023 test_range_source_low-high
[default range target low]
unconfined_u:object_r:mnt_t:s0 test_range_target_low
[default range target high]
unconfined_u:object_r:mnt_t:s0:c0.c512 test_range_target_high
[default range target low-high]
unconfined_u:object_r:mnt_t:s0-s0:c0.c512 test_range_target_low-high
Signed-off-by: Eric Paris <eparis@redhat.com>
There is no reason the DAC perms on reading the policy file need to be root
only. There are selinux checks which should control this access.
Signed-off-by: Eric Paris <eparis@redhat.com>
I am about to remove the struct user_namespace reference from struct user_struct.
So keep an explicit track of the parent user namespace.
Take advantage of this new reference and replace instances of user_ns->creator->user_ns
with user_ns->parent.
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
struct user_struct will shortly loose it's user_ns reference
so make the cred user_ns reference a proper reference complete
with reference counting.
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Optimize performance and prepare for the removal of the user_ns reference
from user_struct. Remove the slow long walk through cred->user->user_ns and
instead go straight to cred->user_ns.
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
It just bloats the audit data structure for no good reason, since the
only time those fields are filled are just before calling the
common_lsm_audit() function, which is also the only user of those
fields.
So just make them be the arguments to common_lsm_audit(), rather than
bloating that structure that is passed around everywhere, and is
initialized in hot paths.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Instead of declaring the entire selinux_audit_data on the stack when we
start an operation on declare it on the stack if we are going to use it.
We know it's usefulness at the end of the security decision and can declare
it there.
Signed-off-by: Eric Paris <eparis@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
After shrinking the common_audit_data stack usage for private LSM data I'm
not going to shrink the data union. To do this I'm going to move anything
larger than 2 void * ptrs to it's own structure and require it to be declared
separately on the calling stack. Thus hot paths which don't need more than
a couple pointer don't have to declare space to hold large unneeded
structures. I could get this down to one void * by dealing with the key
struct and the struct path. We'll see if that is helpful after taking care of
networking.
Signed-off-by: Eric Paris <eparis@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Linus found that the gigantic size of the common audit data caused a big
perf hit on something as simple as running stat() in a loop. This patch
requires LSMs to declare the LSM specific portion separately rather than
doing it in a union. Thus each LSM can be responsible for shrinking their
portion and don't have to pay a penalty just because other LSMs have a
bigger space requirement.
Signed-off-by: Eric Paris <eparis@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Convert debug, freezer, cpuset, cpu_cgroup, cpuacct, net_prio, blkio,
net_cls and device controllers to use the new cftype based interface.
Termination entry is added to cftype arrays and populate callbacks are
replaced with cgroup_subsys->base_cftypes initializations.
This is functionally identical transformation. There shouldn't be any
visible behavior change.
memcg is rather special and will be converted separately.
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Li Zefan <lizf@cn.fujitsu.com>
Cc: Paul Menage <paul@paulmenage.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Vivek Goyal <vgoyal@redhat.com>
Now that all the slow-path code is gone from these functions, we can
inline them into the main caller - avc_has_perm_flags().
Now the compiler can see that 'avc' is allocated on the stack for this
case, which helps register pressure a bit. It also actually shrinks the
total stack frame, because the stack frame that avc_has_perm_flags()
always needed (for that 'avc' allocation) is now sufficient for the
inlined functions too.
Inlining isn't bad - but mindless inlining of cold code (see the
previous commit) is.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
The selinux AVC paths remain some of the hottest (and deepest) codepaths
at filename lookup time, and we make it worse by having the slow path
cases take up I$ and stack space even when they don't trigger. Gcc
tends to always want to inline functions that are just called once -
never mind that this might make for slower and worse code in the caller.
So this tries to improve on it a bit by making the slow-path cases
explicitly separate functions that are marked noinline, causing gcc to
at least no longer allocate stack space for them unless they are
actually called. It also seems to help register allocation a tiny bit,
since gcc now doesn't take the slow case code into account.
Uninlining the slow path may also allow us to inline the remaining hot
path into the one caller that actually matters: avc_has_perm_flags().
I'll have to look at that separately, but both avc_audit() and
avc_has_perm_noaudit() are now small and lean enough that inlining them
may make sense.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Remove all #inclusions of asm/system.h preparatory to splitting and killing
it. Performed with the following command:
perl -p -i -e 's!^#\s*include\s*<asm/system[.]h>.*\n!!' `grep -Irl '^#\s*include\s*<asm/system[.]h>' *`
Signed-off-by: David Howells <dhowells@redhat.com>
Fix failure in aa_change_onexec api when the request is made from a confined
task. This failure was caused by two problems
The AA_MAY_ONEXEC perm was not being mapped correctly for this case.
The executable name was being checked as second time instead of using the
requested onexec profile name, which may not be the same as the exec
profile name. This mistake can not be exploited to grant extra permission
because of the above flaw where the ONEXEC permission was not being mapped
so it will not be granted.
BugLink: http://bugs.launchpad.net/bugs/963756
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
selinux/xfrm.h needs to #include net/flow.h or else suffer:
In file included from security/selinux/ss/services.c:69:0:
security/selinux/include/xfrm.h: In function 'selinux_xfrm_notify_policyload':
security/selinux/include/xfrm.h:53:14: error: 'flow_cache_genid' undeclared (first use in this function)
security/selinux/include/xfrm.h:53:14: note: each undeclared identifier is reported only once for each function it appears in
Signed-off-by: David Howells <dhowells@redhat.com>
No functional changes. It is not sane to use UMH_KILLABLE with enum
umh_wait, but obviously we do not want another argument in
call_usermodehelper_* helpers. Kill this enum, use the plain int.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Tejun Heo <tj@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
avc_audit() did a lot of jumping around and had a big stack frame, all
for the uncommon case.
Split up the uncommon case (which we really can't make go fast anyway)
into its own slow function, and mark the conditional branches
appropriately for the common likely case.
This causes avc_audit() to no longer show up as one of the hottest
functions on the branch profiles (the new "perf -b" thing), and makes
the cycle profiles look really nice and dense too.
The whole audit path is still annoyingly very much one of the biggest
costs of name lookup, so these things are worth optimizing for. I wish
we could just tell people to turn it off, but realistically we do need
it: we just need to make sure that the overhead of the necessary evil is
as low as possible.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Call sequence:
tomoyo_write_domain() --> tomoyo_delete_domain()
In 'tomoyo_delete_domain', return -EINTR if locking attempt is
interrupted by signal.
At present it returns success to its caller 'tomoyo_write_domain()'
even though domain is not deleted. 'tomoyo_write_domain()' assumes
domain is deleted and returns success to its caller. This is wrong behaviour.
'tomoyo_write_domain' should return error from tomoyo_delete_domain() to its
caller.
Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Add the base support for the new policy extensions. This does not bring
any additional functionality, or change current semantics.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
"struct file_operations"->poll() expects "unsigned int" return value.
All files in /sys/kernel/security/tomoyo/ directory other than
/sys/kernel/security/tomoyo/query and /sys/kernel/security/tomoyo/audit should
return POLLIN | POLLRDNORM | POLLOUT | POLLWRNORM rather than -ENOSYS.
Also, /sys/kernel/security/tomoyo/query and /sys/kernel/security/tomoyo/audit
should return POLLOUT | POLLWRNORM rather than 0 when there is no data to read.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Move the path name lookup failure messages into the main path name lookup
routine, as the information is useful in more than just aa_path_perm.
Also rename aa_get_name to aa_path_name as it is not getting a reference
counted object with a corresponding put fn.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
Update aa_dfa_match so that it doesn't result in an input string being
walked twice (once to get its length and another time to match)
Add a single step functions
aa_dfa_next
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
When __d_path and d_absolute_path fail due to the name being outside of
the current namespace no name is reported. Use dentry_path to provide
some hint as to which file was being accessed.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
Post unpacking of policy a verification pass is made on x transition
indexes. When this fails a call to audit_iface is made resulting in an
oops, because audit_iface is expecting a valid buffer position but
since the failure comes from post unpack verification there is none.
Make the position argument optional so that audit_iface can be called
from post unpack verification.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The returning of -ESATLE when a path lookup fails as disconnected is wrong.
Since AppArmor is rejecting the access return -EACCES instead.
This also fixes a bug in complain (learning) mode where disconnected paths
are denied because -ESTALE errors are not ignored causing failures that
can change application behavior.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The test for "if (cred->request_key_auth->flags & KEY_FLAG_REVOKED) {"
should actually testing that the (1 << KEY_FLAG_REVOKED) bit is set.
The current code actually checks for KEY_FLAG_DEAD.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
The keyctl_set_timeout function isn't exported to other parts of the
kernel, but I want to use it for the NFS idmapper. I already have the
key, but I wanted a generic way to set the timeout.
Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Userspace can pass in arbitrary combinations of MS_* flags to mount().
If both MS_BIND and one of MS_SHARED/MS_PRIVATE/MS_SLAVE/MS_UNBINDABLE are
passed, device name which should be checked for MS_BIND was not checked because
MS_SHARED/MS_PRIVATE/MS_SLAVE/MS_UNBINDABLE had higher priority than MS_BIND.
If both one of MS_BIND/MS_MOVE and MS_REMOUNT are passed, device name which
should not be checked for MS_REMOUNT was checked because MS_BIND/MS_MOVE had
higher priority than MS_REMOUNT.
Fix these bugs by changing priority to MS_REMOUNT -> MS_BIND ->
MS_SHARED/MS_PRIVATE/MS_SLAVE/MS_UNBINDABLE -> MS_MOVE as with do_mount() does.
Also, unconditionally return -EINVAL if more than one of
MS_SHARED/MS_PRIVATE/MS_SLAVE/MS_UNBINDABLE is passed so that TOMOYO will not
generate inaccurate audit logs, for commit 7a2e8a8f "VFS: Sanity check mount
flags passed to change_mnt_propagation()" clarified that these flags must be
exclusively passed.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Eric Paris