BugLink: http://bugs.launchpad.net/bugs/1235977
The profile introspection seq file has a locking bug when policy is viewed
from a virtual root (task in a policy namespace), introspection from the
real root is not affected.
The test for root
while (parent) {
is correct for the real root, but incorrect for tasks in a policy namespace.
This allows the task to walk backup the policy tree past its virtual root
causing it to be unlocked before the virtual root should be in the p_stop
fn.
This results in the following lockdep back trace:
[ 78.479744] [ BUG: bad unlock balance detected! ]
[ 78.479792] 3.11.0-11-generic #17 Not tainted
[ 78.479838] -------------------------------------
[ 78.479885] grep/2223 is trying to release lock (&ns->lock) at:
[ 78.479952] [<ffffffff817bf3be>] mutex_unlock+0xe/0x10
[ 78.480002] but there are no more locks to release!
[ 78.480037]
[ 78.480037] other info that might help us debug this:
[ 78.480037] 1 lock held by grep/2223:
[ 78.480037] #0: (&p->lock){+.+.+.}, at: [<ffffffff812111bd>] seq_read+0x3d/0x3d0
[ 78.480037]
[ 78.480037] stack backtrace:
[ 78.480037] CPU: 0 PID: 2223 Comm: grep Not tainted 3.11.0-11-generic #17
[ 78.480037] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 78.480037] ffffffff817bf3be ffff880007763d60 ffffffff817b97ef ffff8800189d2190
[ 78.480037] ffff880007763d88 ffffffff810e1c6e ffff88001f044730 ffff8800189d2190
[ 78.480037] ffffffff817bf3be ffff880007763e00 ffffffff810e5bd6 0000000724fe56b7
[ 78.480037] Call Trace:
[ 78.480037] [<ffffffff817bf3be>] ? mutex_unlock+0xe/0x10
[ 78.480037] [<ffffffff817b97ef>] dump_stack+0x54/0x74
[ 78.480037] [<ffffffff810e1c6e>] print_unlock_imbalance_bug+0xee/0x100
[ 78.480037] [<ffffffff817bf3be>] ? mutex_unlock+0xe/0x10
[ 78.480037] [<ffffffff810e5bd6>] lock_release_non_nested+0x226/0x300
[ 78.480037] [<ffffffff817bf2fe>] ? __mutex_unlock_slowpath+0xce/0x180
[ 78.480037] [<ffffffff817bf3be>] ? mutex_unlock+0xe/0x10
[ 78.480037] [<ffffffff810e5d5c>] lock_release+0xac/0x310
[ 78.480037] [<ffffffff817bf2b3>] __mutex_unlock_slowpath+0x83/0x180
[ 78.480037] [<ffffffff817bf3be>] mutex_unlock+0xe/0x10
[ 78.480037] [<ffffffff81376c91>] p_stop+0x51/0x90
[ 78.480037] [<ffffffff81211408>] seq_read+0x288/0x3d0
[ 78.480037] [<ffffffff811e9d9e>] vfs_read+0x9e/0x170
[ 78.480037] [<ffffffff811ea8cc>] SyS_read+0x4c/0xa0
[ 78.480037] [<ffffffff817ccc9d>] system_call_fastpath+0x1a/0x1f
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Use the shash interface, rather than the hash interface, when hashing
AppArmor profiles. The shash interface does not use scatterlists and it
is a better fit for what AppArmor needs.
This fixes a kernel paging BUG when aa_calc_profile_hash() is passed a
buffer from vmalloc(). The hash interface requires callers to handle
vmalloc() buffers differently than what AppArmor was doing. Due to
vmalloc() memory not being physically contiguous, each individual page
behind the buffer must be assigned to a scatterlist with sg_set_page()
and then the scatterlist passed to crypto_hash_update().
The shash interface does not have that limitation and allows vmalloc()
and kmalloc() buffers to be handled in the same manner.
BugLink: https://launchpad.net/bugs/1216294/
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=62261
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Pull security subsystem updates from James Morris:
"Nothing major for this kernel, just maintenance updates"
* 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: (21 commits)
apparmor: add the ability to report a sha1 hash of loaded policy
apparmor: export set of capabilities supported by the apparmor module
apparmor: add the profile introspection file to interface
apparmor: add an optional profile attachment string for profiles
apparmor: add interface files for profiles and namespaces
apparmor: allow setting any profile into the unconfined state
apparmor: make free_profile available outside of policy.c
apparmor: rework namespace free path
apparmor: update how unconfined is handled
apparmor: change how profile replacement update is done
apparmor: convert profile lists to RCU based locking
apparmor: provide base for multiple profiles to be replaced at once
apparmor: add a features/policy dir to interface
apparmor: enable users to query whether apparmor is enabled
apparmor: remove minimum size check for vmalloc()
Smack: parse multiple rules per write to load2, up to PAGE_SIZE-1 bytes
Smack: network label match fix
security: smack: add a hash table to quicken smk_find_entry()
security: smack: fix memleak in smk_write_rules_list()
xattr: Constify ->name member of "struct xattr".
...
The apparmor module parameters for param_ops_aabool and
param_ops_aalockpolicy are both based off of the param_ops_bool,
and can handle a NULL value passed in as val. Have it enable the
new KERNEL_PARAM_FL_NOARGS flag to allow the parameters to be set
without having to state "=y" or "=1".
Cc: John Johansen <john.johansen@canonical.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Provide userspace the ability to introspect a sha1 hash value for each
profile currently loaded.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Add the dynamic namespace relative profiles file to the interace, to allow
introspection of loaded profiles and their modes.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
Add the ability to take in and report a human readable profile attachment
string for profiles so that attachment specifications can be easily
inspected.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Add basic interface files to access namespace and profile information.
The interface files are created when a profile is loaded and removed
when the profile or namespace is removed.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Allow emulating the default profile behavior from boot, by allowing
loading of a profile in the unconfined state into a new NS.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
namespaces now completely use the unconfined profile to track the
refcount and rcu freeing cycle. So rework the code to simplify (track
everything through the profile path right up to the end), and move the
rcu_head from policy base to profile as the namespace no longer needs
it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
ns->unconfined is being used read side without locking, nor rcu but is
being updated when a namespace is removed. This works for the root ns
which is never removed but has a race window and can cause failures when
children namespaces are removed.
Also ns and ns->unconfined have a circular refcounting dependency that
is problematic and must be broken. Currently this is done incorrectly
when the namespace is destroyed.
Fix this by forward referencing unconfined via the replacedby infrastructure
instead of directly updating the ns->unconfined pointer.
Remove the circular refcount dependency by making the ns and its unconfined
profile share the same refcount.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
remove the use of replaced by chaining and move to profile invalidation
and lookup to handle task replacement.
Replacement chaining can result in large chains of profiles being pinned
in memory when one profile in the chain is use. With implicit labeling
this will be even more of a problem, so move to a direct lookup method.
Signed-off-by: John Johansen <john.johansen@canonical.com>
previously profiles had to be loaded one at a time, which could result
in cases where a replacement of a set would partially succeed, and then fail
resulting in inconsistent policy.
Allow multiple profiles to replaced "atomically" so that the replacement
either succeeds or fails for the entire set of profiles.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add a policy directory to features to contain features that can affect
policy compilation but do not affect mediation. Eg of such features would
be types of dfa compression supported, etc.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
This is a follow-up to commit b5b3ee6c "apparmor: no need to delay vfree()".
Since vmalloc() will do "size = PAGE_ALIGN(size);",
we don't need to check for "size >= sizeof(struct work_struct)".
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: John Johansen <john.johansen@canonical.com>
vfree() can be called from interrupt contexts now
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Acked-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
currently apparmor name parsing is only correctly handling
:<NS>:<profile>
but
:<NS>://<profile>
is also a valid form and what is exported to userspace.
Signed-off-by: John Johansen <john.johansen@canonical.com>
the exec file isn't processing its command arg. It should only set be
responding to a command of exec.
Also cleanup setprocattr some more while we are at it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
smatch reports
error: potential NULL dereference 'ns'.
this can not actually occur because it relies on aa_split_fqname setting
both ns_name and name as null but ns_name will actually always have a
value in this case.
so remove the unnecessary if (ns_name) conditional that is resulting
in the false positive further down.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The audit type table is missing a comma so that KILLED comes out as
KILLEDAUTO.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The top 8 bits of the base field have never been used, in fact can't
be used, by the current 'dfa16' format. However they will be used in the
future as flags, so mask them off when using base as an index value.
Note: the use of the top 8 bits, without masking is trapped by the verify
checks that base entries are within the size bounds.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
Move the free_profile fn ahead of aa_alloc_profile so it can be used
in aa_alloc_profile without a forward declaration.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
The sid is not going to be a direct property of a profile anymore, instead
it will be directly related to the label, and the profile will pickup
a label back reference.
For null-profiles replace the use of sid with a per namespace unique
id.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
Instead of limiting the setting of the processes limits to current,
relax this to tasks confined by the same profile, as the apparmor
controls for rlimits are at a profile level granularity.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The "permipc" command is unused and unfinished, remove it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
-ESTALE used to be incorrectly used to indicate a disconnected path, when
name lookup failed. This was fixed in commit e1b0e444 to correctly return
-EACCESS, but the error to failure message mapping was not correctly updated
to reflect this change.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
When policy specifies a transition to a profile that is not currently
loaded, it result in exec being denied. However the failure is not being
audited correctly because the audit code is treating this as an allowed
permission and thus not reporting it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
BugLink: http://bugs.launchpad.net/bugs/1056078
Profile replacement can cause long chains of profiles to build up when
the profile being replaced is pinned. When the pinned profile is finally
freed, it puts the reference to its replacement, which may in turn nest
another call to free_profile on the stack. Because this may happen for
each profile in the replacedby chain this can result in a recusion that
causes the stack to overflow.
Break this nesting by directly walking the chain of replacedby profiles
(ie. use iteration instead of recursion to free the list). This results
in at most 2 levels of free_profile being called, while freeing a
replacedby chain.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
The capability defines have moved causing the auto generated names
of capabilities that apparmor uses in logging to be incorrect.
Fix the autogenerated table source to uapi/linux/capability.h
Reported-by: YanHong <clouds.yan@gmail.com>
Reported-by: Krzysztof Kolasa <kkolasa@winsoft.pl>
Analyzed-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: David Howells <dhowells@redhat.com>
Acked-by: James Morris <james.l.morris@oracle.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Michael Kerrisk <mtk.manpages@gmail.com>
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Dave Jones <davej@redhat.com>
Pull user namespace changes from Eric Biederman:
"This is a mostly modest set of changes to enable basic user namespace
support. This allows the code to code to compile with user namespaces
enabled and removes the assumption there is only the initial user
namespace. Everything is converted except for the most complex of the
filesystems: autofs4, 9p, afs, ceph, cifs, coda, fuse, gfs2, ncpfs,
nfs, ocfs2 and xfs as those patches need a bit more review.
The strategy is to push kuid_t and kgid_t values are far down into
subsystems and filesystems as reasonable. Leaving the make_kuid and
from_kuid operations to happen at the edge of userspace, as the values
come off the disk, and as the values come in from the network.
Letting compile type incompatible compile errors (present when user
namespaces are enabled) guide me to find the issues.
The most tricky areas have been the places where we had an implicit
union of uid and gid values and were storing them in an unsigned int.
Those places were converted into explicit unions. I made certain to
handle those places with simple trivial patches.
Out of that work I discovered we have generic interfaces for storing
quota by projid. I had never heard of the project identifiers before.
Adding full user namespace support for project identifiers accounts
for most of the code size growth in my git tree.
Ultimately there will be work to relax privlige checks from
"capable(FOO)" to "ns_capable(user_ns, FOO)" where it is safe allowing
root in a user names to do those things that today we only forbid to
non-root users because it will confuse suid root applications.
While I was pushing kuid_t and kgid_t changes deep into the audit code
I made a few other cleanups. I capitalized on the fact we process
netlink messages in the context of the message sender. I removed
usage of NETLINK_CRED, and started directly using current->tty.
Some of these patches have also made it into maintainer trees, with no
problems from identical code from different trees showing up in
linux-next.
After reading through all of this code I feel like I might be able to
win a game of kernel trivial pursuit."
Fix up some fairly trivial conflicts in netfilter uid/git logging code.
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace: (107 commits)
userns: Convert the ufs filesystem to use kuid/kgid where appropriate
userns: Convert the udf filesystem to use kuid/kgid where appropriate
userns: Convert ubifs to use kuid/kgid
userns: Convert squashfs to use kuid/kgid where appropriate
userns: Convert reiserfs to use kuid and kgid where appropriate
userns: Convert jfs to use kuid/kgid where appropriate
userns: Convert jffs2 to use kuid and kgid where appropriate
userns: Convert hpfs to use kuid and kgid where appropriate
userns: Convert btrfs to use kuid/kgid where appropriate
userns: Convert bfs to use kuid/kgid where appropriate
userns: Convert affs to use kuid/kgid wherwe appropriate
userns: On alpha modify linux_to_osf_stat to use convert from kuids and kgids
userns: On ia64 deal with current_uid and current_gid being kuid and kgid
userns: On ppc convert current_uid from a kuid before printing.
userns: Convert s390 getting uid and gid system calls to use kuid and kgid
userns: Convert s390 hypfs to use kuid and kgid where appropriate
userns: Convert binder ipc to use kuids
userns: Teach security_path_chown to take kuids and kgids
userns: Add user namespace support to IMA
userns: Convert EVM to deal with kuids and kgids in it's hmac computation
...
Don't make the security modules deal with raw user space uid and
gids instead pass in a kuid_t and a kgid_t so that security modules
only have to deal with internal kernel uids and gids.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: James Morris <james.l.morris@oracle.com>
Cc: John Johansen <john.johansen@canonical.com>
Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Cc: John Johansen <john.johansen@canonical.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Commit 4fdef2183e ("AppArmor: Cleanup make
file to remove cruft and make it easier to read") removed all traces of
af_names.h from the tree. Remove its entry in AppArmor's .gitignore file
too.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>