From 19e49834d22c2271ed1f4a03aaa4b74986447fb4 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 4 Oct 2013 12:54:11 -0700 Subject: selinux: remove 'flags' parameter from inode_has_perm Every single user passes in '0'. I think we had non-zero users back in some stone age when selinux_inode_permission() was implemented in terms of inode_has_perm(), but that complicated case got split up into a totally separate code-path so that we could optimize the much simpler special cases. See commit 2e33405785d3 ("SELinux: delay initialization of audit data in selinux_inode_permission") for example. Signed-off-by: Linus Torvalds --- security/selinux/hooks.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a5091ec..9678232 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1525,8 +1525,7 @@ static int task_has_system(struct task_struct *tsk, static int inode_has_perm(const struct cred *cred, struct inode *inode, u32 perms, - struct common_audit_data *adp, - unsigned flags) + struct common_audit_data *adp) { struct inode_security_struct *isec; u32 sid; @@ -1539,7 +1538,7 @@ static int inode_has_perm(const struct cred *cred, sid = cred_sid(cred); isec = inode->i_security; - return avc_has_perm_flags(sid, isec->sid, isec->sclass, perms, adp, flags); + return avc_has_perm(sid, isec->sid, isec->sclass, perms, adp); } /* Same as inode_has_perm, but pass explicit audit data containing @@ -1554,7 +1553,7 @@ static inline int dentry_has_perm(const struct cred *cred, ad.type = LSM_AUDIT_DATA_DENTRY; ad.u.dentry = dentry; - return inode_has_perm(cred, inode, av, &ad, 0); + return inode_has_perm(cred, inode, av, &ad); } /* Same as inode_has_perm, but pass explicit audit data containing @@ -1569,7 +1568,7 @@ static inline int path_has_perm(const struct cred *cred, ad.type = LSM_AUDIT_DATA_PATH; ad.u.path = *path; - return inode_has_perm(cred, inode, av, &ad, 0); + return inode_has_perm(cred, inode, av, &ad); } /* Same as path_has_perm, but uses the inode from the file struct. */ @@ -1581,7 +1580,7 @@ static inline int file_path_has_perm(const struct cred *cred, ad.type = LSM_AUDIT_DATA_PATH; ad.u.path = file->f_path; - return inode_has_perm(cred, file_inode(file), av, &ad, 0); + return inode_has_perm(cred, file_inode(file), av, &ad); } /* Check whether a task can use an open file descriptor to @@ -1617,7 +1616,7 @@ static int file_has_perm(const struct cred *cred, /* av is zero if only checking access to the descriptor. */ rc = 0; if (av) - rc = inode_has_perm(cred, inode, av, &ad, 0); + rc = inode_has_perm(cred, inode, av, &ad); out: return rc; -- cgit v1.1 From cb4fbe5703be51f8a2dff4052b1901941ab99e12 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 4 Oct 2013 12:57:22 -0700 Subject: selinux: avc_has_perm_flags has no more users .. so get rid of it. The only indirect users were all the avc_has_perm() callers which just expanded to have a zero flags argument. Signed-off-by: Linus Torvalds --- security/selinux/avc.c | 9 +++------ security/selinux/include/avc.h | 14 +++----------- 2 files changed, 6 insertions(+), 17 deletions(-) (limited to 'security') diff --git a/security/selinux/avc.c b/security/selinux/avc.c index dad36a6..e720f72 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -746,7 +746,6 @@ inline int avc_has_perm_noaudit(u32 ssid, u32 tsid, * @tclass: target security class * @requested: requested permissions, interpreted based on @tclass * @auditdata: auxiliary audit data - * @flags: VFS walk flags * * Check the AVC to determine whether the @requested permissions are granted * for the SID pair (@ssid, @tsid), interpreting the permissions @@ -756,17 +755,15 @@ inline int avc_has_perm_noaudit(u32 ssid, u32 tsid, * permissions are granted, -%EACCES if any permissions are denied, or * another -errno upon other errors. */ -int avc_has_perm_flags(u32 ssid, u32 tsid, u16 tclass, - u32 requested, struct common_audit_data *auditdata, - unsigned flags) +int avc_has_perm(u32 ssid, u32 tsid, u16 tclass, + u32 requested, struct common_audit_data *auditdata) { struct av_decision avd; int rc, rc2; rc = avc_has_perm_noaudit(ssid, tsid, tclass, requested, 0, &avd); - rc2 = avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata, - flags); + rc2 = avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata, 0); if (rc2) return rc2; return rc; diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h index 92d0ab5..e30657b 100644 --- a/security/selinux/include/avc.h +++ b/security/selinux/include/avc.h @@ -147,17 +147,9 @@ int avc_has_perm_noaudit(u32 ssid, u32 tsid, unsigned flags, struct av_decision *avd); -int avc_has_perm_flags(u32 ssid, u32 tsid, - u16 tclass, u32 requested, - struct common_audit_data *auditdata, - unsigned); - -static inline int avc_has_perm(u32 ssid, u32 tsid, - u16 tclass, u32 requested, - struct common_audit_data *auditdata) -{ - return avc_has_perm_flags(ssid, tsid, tclass, requested, auditdata, 0); -} +int avc_has_perm(u32 ssid, u32 tsid, + u16 tclass, u32 requested, + struct common_audit_data *auditdata); u32 avc_policy_seqno(void); -- cgit v1.1 From ab3540626435c01e08fe58ce544311a78430f112 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 4 Oct 2013 14:05:38 -0700 Subject: selinux: remove 'flags' parameter from avc_audit() Now avc_audit() has no more users with that parameter. Remove it. Signed-off-by: Linus Torvalds --- security/selinux/avc.c | 2 +- security/selinux/hooks.c | 2 +- security/selinux/include/avc.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/selinux/avc.c b/security/selinux/avc.c index e720f72..fc3e662 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -763,7 +763,7 @@ int avc_has_perm(u32 ssid, u32 tsid, u16 tclass, rc = avc_has_perm_noaudit(ssid, tsid, tclass, requested, 0, &avd); - rc2 = avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata, 0); + rc2 = avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata); if (rc2) return rc2; return rc; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9678232..5b52310 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1502,7 +1502,7 @@ static int cred_has_capability(const struct cred *cred, rc = avc_has_perm_noaudit(sid, sid, sclass, av, 0, &avd); if (audit == SECURITY_CAP_AUDIT) { - int rc2 = avc_audit(sid, sid, sclass, av, &avd, rc, &ad, 0); + int rc2 = avc_audit(sid, sid, sclass, av, &avd, rc, &ad); if (rc2) return rc2; } diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h index e30657b..f53ee3c 100644 --- a/security/selinux/include/avc.h +++ b/security/selinux/include/avc.h @@ -130,7 +130,7 @@ static inline int avc_audit(u32 ssid, u32 tsid, u16 tclass, u32 requested, struct av_decision *avd, int result, - struct common_audit_data *a, unsigned flags) + struct common_audit_data *a) { u32 audited, denied; audited = avc_audit_required(requested, avd, result, 0, &denied); @@ -138,7 +138,7 @@ static inline int avc_audit(u32 ssid, u32 tsid, return 0; return slow_avc_audit(ssid, tsid, tclass, requested, audited, denied, - a, flags); + a, 0); } #define AVC_STRICT 1 /* Ignore permissive mode. */ -- cgit v1.1 From 5cb3e91ebd0405519795f243adbfc4ed2a6fe53f Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 14 Oct 2013 11:44:34 -0700 Subject: apparmor: fix memleak of the profile hash BugLink: http://bugs.launchpad.net/bugs/1235523 This fixes the following kmemleak trace: unreferenced object 0xffff8801e8c35680 (size 32): comm "apparmor_parser", pid 691, jiffies 4294895667 (age 13230.876s) hex dump (first 32 bytes): e0 d3 4e b5 ac 6d f4 ed 3f cb ee 48 1c fd 40 cf ..N..m..?..H..@. 5b cc e9 93 00 00 00 00 00 00 00 00 00 00 00 00 [............... backtrace: [] kmemleak_alloc+0x4e/0xb0 [] __kmalloc+0x103/0x290 [] aa_calc_profile_hash+0x6c/0x150 [] aa_unpack+0x39d/0xd50 [] aa_replace_profiles+0x3d/0xd80 [] profile_replace+0x37/0x50 [] vfs_write+0xbd/0x1e0 [] SyS_write+0x4c/0xa0 [] system_call_fastpath+0x1a/0x1f [] 0xffffffffffffffff Signed-off-by: John Johansen Signed-off-by: James Morris --- security/apparmor/policy.c | 1 + 1 file changed, 1 insertion(+) (limited to 'security') diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 345bec0..705c287 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -610,6 +610,7 @@ void aa_free_profile(struct aa_profile *profile) aa_put_dfa(profile->policy.dfa); aa_put_replacedby(profile->replacedby); + kzfree(profile->hash); kzfree(profile); } -- cgit v1.1 From ed2c7da3a40c58410508fe24e12d03e508d7ec01 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 14 Oct 2013 11:46:27 -0700 Subject: apparmor: fix bad lock balance when introspecting policy 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] [] 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: [] 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] [] ? mutex_unlock+0xe/0x10 [ 78.480037] [] dump_stack+0x54/0x74 [ 78.480037] [] print_unlock_imbalance_bug+0xee/0x100 [ 78.480037] [] ? mutex_unlock+0xe/0x10 [ 78.480037] [] lock_release_non_nested+0x226/0x300 [ 78.480037] [] ? __mutex_unlock_slowpath+0xce/0x180 [ 78.480037] [] ? mutex_unlock+0xe/0x10 [ 78.480037] [] lock_release+0xac/0x310 [ 78.480037] [] __mutex_unlock_slowpath+0x83/0x180 [ 78.480037] [] mutex_unlock+0xe/0x10 [ 78.480037] [] p_stop+0x51/0x90 [ 78.480037] [] seq_read+0x288/0x3d0 [ 78.480037] [] vfs_read+0x9e/0x170 [ 78.480037] [] SyS_read+0x4c/0xa0 [ 78.480037] [] system_call_fastpath+0x1a/0x1f Signed-off-by: John Johansen Signed-off-by: James Morris --- security/apparmor/apparmorfs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'security') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 95c2b26..7db9954 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -580,15 +580,13 @@ static struct aa_namespace *__next_namespace(struct aa_namespace *root, /* check if the next ns is a sibling, parent, gp, .. */ parent = ns->parent; - while (parent) { + while (ns != root) { mutex_unlock(&ns->lock); next = list_entry_next(ns, base.list); if (!list_entry_is_head(next, &parent->sub_ns, base.list)) { mutex_lock(&next->lock); return next; } - if (parent == root) - return NULL; ns = parent; parent = parent->parent; } -- cgit v1.1