From 9548906b2bb7ff09e12c013a55d669bef2c8e121 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Thu, 25 Jul 2013 05:44:02 +0900 Subject: xattr: Constify ->name member of "struct xattr". Since everybody sets kstrdup()ed constant string to "struct xattr"->name but nobody modifies "struct xattr"->name , we can omit kstrdup() and its failure checking by constifying ->name member of "struct xattr". Signed-off-by: Tetsuo Handa Reviewed-by: Joel Becker [ocfs2] Acked-by: Serge E. Hallyn Acked-by: Casey Schaufler Acked-by: Mimi Zohar Reviewed-by: Paul Moore Tested-by: Paul Moore Acked-by: Eric Paris Signed-off-by: James Morris --- security/capability.c | 2 +- security/integrity/evm/evm_main.c | 2 +- security/security.c | 8 +++----- security/selinux/hooks.c | 17 ++++++----------- security/smack/smack_lsm.c | 9 +++------ 5 files changed, 14 insertions(+), 24 deletions(-) (limited to 'security') diff --git a/security/capability.c b/security/capability.c index 32b515766df1..dbeb9bc27b24 100644 --- a/security/capability.c +++ b/security/capability.c @@ -129,7 +129,7 @@ static void cap_inode_free_security(struct inode *inode) } static int cap_inode_init_security(struct inode *inode, struct inode *dir, - const struct qstr *qstr, char **name, + const struct qstr *qstr, const char **name, void **value, size_t *len) { return -EOPNOTSUPP; diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index df0fa451a871..af9b6852f4e1 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -418,7 +418,7 @@ int evm_inode_init_security(struct inode *inode, evm_xattr->value = xattr_data; evm_xattr->value_len = sizeof(*xattr_data); - evm_xattr->name = kstrdup(XATTR_EVM_SUFFIX, GFP_NOFS); + evm_xattr->name = XATTR_EVM_SUFFIX; return 0; out: kfree(xattr_data); diff --git a/security/security.c b/security/security.c index 94b35aef6871..4dc31f4f2700 100644 --- a/security/security.c +++ b/security/security.c @@ -348,10 +348,10 @@ int security_inode_init_security(struct inode *inode, struct inode *dir, if (unlikely(IS_PRIVATE(inode))) return 0; - memset(new_xattrs, 0, sizeof new_xattrs); if (!initxattrs) return security_ops->inode_init_security(inode, dir, qstr, NULL, NULL, NULL); + memset(new_xattrs, 0, sizeof(new_xattrs)); lsm_xattr = new_xattrs; ret = security_ops->inode_init_security(inode, dir, qstr, &lsm_xattr->name, @@ -366,16 +366,14 @@ int security_inode_init_security(struct inode *inode, struct inode *dir, goto out; ret = initxattrs(inode, new_xattrs, fs_data); out: - for (xattr = new_xattrs; xattr->name != NULL; xattr++) { - kfree(xattr->name); + for (xattr = new_xattrs; xattr->value != NULL; xattr++) kfree(xattr->value); - } return (ret == -EOPNOTSUPP) ? 0 : ret; } EXPORT_SYMBOL(security_inode_init_security); int security_old_inode_init_security(struct inode *inode, struct inode *dir, - const struct qstr *qstr, char **name, + const struct qstr *qstr, const char **name, void **value, size_t *len) { if (unlikely(IS_PRIVATE(inode))) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index c956390a9136..a5091ec06aa6 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2587,7 +2587,8 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode, } static int selinux_inode_init_security(struct inode *inode, struct inode *dir, - const struct qstr *qstr, char **name, + const struct qstr *qstr, + const char **name, void **value, size_t *len) { const struct task_security_struct *tsec = current_security(); @@ -2595,7 +2596,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, struct superblock_security_struct *sbsec; u32 sid, newsid, clen; int rc; - char *namep = NULL, *context; + char *context; dsec = dir->i_security; sbsec = dir->i_sb->s_security; @@ -2631,19 +2632,13 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, if (!ss_initialized || !(sbsec->flags & SE_SBLABELSUPP)) return -EOPNOTSUPP; - if (name) { - namep = kstrdup(XATTR_SELINUX_SUFFIX, GFP_NOFS); - if (!namep) - return -ENOMEM; - *name = namep; - } + if (name) + *name = XATTR_SELINUX_SUFFIX; if (value && len) { rc = security_sid_to_context_force(newsid, &context, &clen); - if (rc) { - kfree(namep); + if (rc) return rc; - } *value = context; *len = clen; } diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 3f7682a387b7..a113a779f00c 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -582,7 +582,7 @@ static void smack_inode_free_security(struct inode *inode) * Returns 0 if it all works out, -ENOMEM if there's no memory */ static int smack_inode_init_security(struct inode *inode, struct inode *dir, - const struct qstr *qstr, char **name, + const struct qstr *qstr, const char **name, void **value, size_t *len) { struct inode_smack *issp = inode->i_security; @@ -591,11 +591,8 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, char *dsp = smk_of_inode(dir); int may; - if (name) { - *name = kstrdup(XATTR_SMACK_SUFFIX, GFP_NOFS); - if (*name == NULL) - return -ENOMEM; - } + if (name) + *name = XATTR_SMACK_SUFFIX; if (value) { rcu_read_lock(); -- cgit v1.2.3 From ca4c3fc24e293719fe7410c4e63da9b6bc633b83 Mon Sep 17 00:00:00 2001 From: "fan.du" Date: Tue, 30 Jul 2013 08:33:53 +0800 Subject: net: split rt_genid for ipv4 and ipv6 Current net name space has only one genid for both IPv4 and IPv6, it has below drawbacks: - Add/delete an IPv4 address will invalidate all IPv6 routing table entries. - Insert/remove XFRM policy will also invalidate both IPv4/IPv6 routing table entries even when the policy is only applied for one address family. Thus, this patch attempt to split one genid for two to cater for IPv4 and IPv6 separately in a fine granularity. Signed-off-by: Fan Du Acked-by: Hannes Frederic Sowa Signed-off-by: David S. Miller --- security/selinux/include/xfrm.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h index 65f67cb0aefb..6713f04e30ba 100644 --- a/security/selinux/include/xfrm.h +++ b/security/selinux/include/xfrm.h @@ -50,8 +50,13 @@ int selinux_xfrm_decode_session(struct sk_buff *skb, u32 *sid, int ckall); static inline void selinux_xfrm_notify_policyload(void) { + struct net *net; + atomic_inc(&flow_cache_genid); - rt_genid_bump(&init_net); + rtnl_lock(); + for_each_net(net) + rt_genid_bump_all(net); + rtnl_unlock(); } #else static inline int selinux_xfrm_enabled(void) -- cgit v1.2.3 From 470043ba995a79a274a5db306856975002a06f19 Mon Sep 17 00:00:00 2001 From: Tomasz Stanislawski Date: Thu, 6 Jun 2013 09:30:50 +0200 Subject: security: smack: fix memleak in smk_write_rules_list() The smack_parsed_rule structure is allocated. If a rule is successfully installed then the last reference to the object is lost. This patch fixes this leak. Moreover smack_parsed_rule is allocated on stack because it no longer needed ofter smk_write_rules_list() is finished. Signed-off-by: Tomasz Stanislawski --- security/smack/smackfs.c | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) (limited to 'security') diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index ab167037b2dd..269b270c6473 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -447,7 +447,7 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf, struct list_head *rule_list, struct mutex *rule_lock, int format) { - struct smack_parsed_rule *rule; + struct smack_parsed_rule rule; char *data; int datalen; int rc = -EINVAL; @@ -479,47 +479,36 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf, goto out; } - rule = kzalloc(sizeof(*rule), GFP_KERNEL); - if (rule == NULL) { - rc = -ENOMEM; - goto out; - } - if (format == SMK_LONG_FMT) { /* * Be sure the data string is terminated. */ data[count] = '\0'; - if (smk_parse_long_rule(data, rule, 1, 0)) - goto out_free_rule; + if (smk_parse_long_rule(data, &rule, 1, 0)) + goto out; } else if (format == SMK_CHANGE_FMT) { data[count] = '\0'; - if (smk_parse_long_rule(data, rule, 1, 1)) - goto out_free_rule; + if (smk_parse_long_rule(data, &rule, 1, 1)) + goto out; } else { /* * More on the minor hack for backward compatibility */ if (count == (SMK_OLOADLEN)) data[SMK_OLOADLEN] = '-'; - if (smk_parse_rule(data, rule, 1)) - goto out_free_rule; + if (smk_parse_rule(data, &rule, 1)) + goto out; } if (rule_list == NULL) { load = 1; - rule_list = &rule->smk_subject->smk_rules; - rule_lock = &rule->smk_subject->smk_rules_lock; + rule_list = &rule.smk_subject->smk_rules; + rule_lock = &rule.smk_subject->smk_rules_lock; } - rc = smk_set_access(rule, rule_list, rule_lock, load); - if (rc == 0) { + rc = smk_set_access(&rule, rule_list, rule_lock, load); + if (rc == 0) rc = count; - goto out; - } - -out_free_rule: - kfree(rule); out: kfree(data); return rc; -- cgit v1.2.3 From 4d7cf4a1f49f76f4069114ee08be75cd68c37c5a Mon Sep 17 00:00:00 2001 From: Tomasz Stanislawski Date: Tue, 11 Jun 2013 14:55:13 +0200 Subject: security: smack: add a hash table to quicken smk_find_entry() Accepted for the smack-next tree after changing the number of slots from 128 to 16. This patch adds a hash table to quicken searching of a smack label by its name. Basically, the patch improves performance of SMACK initialization. Parsing of rules involves translation from a string to a smack_known (aka label) entity which is done in smk_find_entry(). The current implementation of the function iterates over a global list of smack_known resulting in O(N) complexity for smk_find_entry(). The total complexity of SMACK initialization becomes O(rules * labels). Therefore it scales quadratically with a complexity of a system. Applying the patch reduced the complexity of smk_find_entry() to O(1) as long as number of label is in hundreds. If the number of labels is increased please update SMACK_HASH_SLOTS constant defined in security/smack/smack.h. Introducing the configuration of this constant with Kconfig or cmdline might be a good idea. The size of the hash table was adjusted experimentally. The rule set used by TIZEN contains circa 17K rules for 500 labels. The table above contains results of SMACK initialization using 'time smackctl apply' bash command. The 'Ref' is a kernel without this patch applied. The consecutive values refers to value of SMACK_HASH_SLOTS. Every measurement was repeated three times to reduce noise. | Ref | 1 | 2 | 4 | 8 | 16 | 32 | 64 | 128 | 256 | 512 -------------------------------------------------------------------------------------------- Run1 | 1.156 | 1.096 | 0.883 | 0.764 | 0.692 | 0.667 | 0.649 | 0.633 | 0.634 | 0.629 | 0.620 Run2 | 1.156 | 1.111 | 0.885 | 0.764 | 0.694 | 0.661 | 0.649 | 0.651 | 0.634 | 0.638 | 0.623 Run3 | 1.160 | 1.107 | 0.886 | 0.764 | 0.694 | 0.671 | 0.661 | 0.638 | 0.631 | 0.624 | 0.638 AVG | 1.157 | 1.105 | 0.885 | 0.764 | 0.693 | 0.666 | 0.653 | 0.641 | 0.633 | 0.630 | 0.627 Surprisingly, a single hlist is slightly faster than a double-linked list. The speed-up saturates near 64 slots. Therefore I chose value 128 to provide some margin if more labels were used. It looks that IO becomes a new bottleneck. Signed-off-by: Tomasz Stanislawski --- security/smack/smack.h | 5 +++++ security/smack/smack_access.c | 29 ++++++++++++++++++++++++++--- security/smack/smack_lsm.c | 12 ++++++------ 3 files changed, 37 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/security/smack/smack.h b/security/smack/smack.h index 339614c76e63..e80597a3048a 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -53,6 +53,7 @@ */ struct smack_known { struct list_head list; + struct hlist_node smk_hashed; char *smk_known; u32 smk_secid; struct netlbl_lsm_secattr smk_netlabel; /* on wire labels */ @@ -222,6 +223,7 @@ char *smk_parse_smack(const char *string, int len); int smk_netlbl_mls(int, char *, struct netlbl_lsm_secattr *, int); char *smk_import(const char *, int); struct smack_known *smk_import_entry(const char *, int); +void smk_insert_entry(struct smack_known *skp); struct smack_known *smk_find_entry(const char *); u32 smack_to_secid(const char *); @@ -247,6 +249,9 @@ extern struct list_head smk_netlbladdr_list; extern struct security_operations smack_ops; +#define SMACK_HASH_SLOTS 16 +extern struct hlist_head smack_known_hash[SMACK_HASH_SLOTS]; + /* * Is the directory transmuting? */ diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index 6a0377f38620..b3b59b1e93d6 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -325,6 +325,25 @@ void smack_log(char *subject_label, char *object_label, int request, DEFINE_MUTEX(smack_known_lock); +struct hlist_head smack_known_hash[SMACK_HASH_SLOTS]; + +/** + * smk_insert_entry - insert a smack label into a hash map, + * + * this function must be called under smack_known_lock + */ +void smk_insert_entry(struct smack_known *skp) +{ + unsigned int hash; + struct hlist_head *head; + + hash = full_name_hash(skp->smk_known, strlen(skp->smk_known)); + head = &smack_known_hash[hash & (SMACK_HASH_SLOTS - 1)]; + + hlist_add_head_rcu(&skp->smk_hashed, head); + list_add_rcu(&skp->list, &smack_known_list); +} + /** * smk_find_entry - find a label on the list, return the list entry * @string: a text string that might be a Smack label @@ -334,12 +353,16 @@ DEFINE_MUTEX(smack_known_lock); */ struct smack_known *smk_find_entry(const char *string) { + unsigned int hash; + struct hlist_head *head; struct smack_known *skp; - list_for_each_entry_rcu(skp, &smack_known_list, list) { + hash = full_name_hash(string, strlen(string)); + head = &smack_known_hash[hash & (SMACK_HASH_SLOTS - 1)]; + + hlist_for_each_entry_rcu(skp, head, smk_hashed) if (strcmp(skp->smk_known, string) == 0) return skp; - } return NULL; } @@ -475,7 +498,7 @@ struct smack_known *smk_import_entry(const char *string, int len) * Make sure that the entry is actually * filled before putting it on the list. */ - list_add_rcu(&skp->list, &smack_known_list); + smk_insert_entry(skp); goto unlockout; } /* diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 3f7682a387b7..ce000a81caf7 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3879,12 +3879,12 @@ static __init void init_smack_known_list(void) /* * Create the known labels list */ - list_add(&smack_known_huh.list, &smack_known_list); - list_add(&smack_known_hat.list, &smack_known_list); - list_add(&smack_known_star.list, &smack_known_list); - list_add(&smack_known_floor.list, &smack_known_list); - list_add(&smack_known_invalid.list, &smack_known_list); - list_add(&smack_known_web.list, &smack_known_list); + smk_insert_entry(&smack_known_huh); + smk_insert_entry(&smack_known_hat); + smk_insert_entry(&smack_known_star); + smk_insert_entry(&smack_known_floor); + smk_insert_entry(&smack_known_invalid); + smk_insert_entry(&smack_known_web); } /** -- cgit v1.2.3 From 677264e8fb73ea35a508700e19ce76c527576d1c Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Fri, 28 Jun 2013 13:47:07 -0700 Subject: Smack: network label match fix The Smack code that matches incoming CIPSO tags with Smack labels reaches through the NetLabel interfaces and compares the network data with the CIPSO header associated with a Smack label. This was done in a ill advised attempt to optimize performance. It works so long as the categories fit in a single capset, but this isn't always the case. This patch changes the Smack code to use the appropriate NetLabel interfaces to compare the incoming CIPSO header with the CIPSO header associated with a label. It will always match the CIPSO headers correctly. Targeted for git://git.gitorious.org/smack-next/kernel.git Signed-off-by: Casey Schaufler --- security/smack/smack.h | 8 ++++++-- security/smack/smack_lsm.c | 30 ++++++++++++++++++++++++------ security/smack/smackfs.c | 2 +- 3 files changed, 31 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/security/smack/smack.h b/security/smack/smack.h index e80597a3048a..076b8e8a51ab 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -168,9 +168,13 @@ struct smk_port_label { #define SMACK_CIPSO_DOI_INVALID -1 /* Not a DOI */ #define SMACK_CIPSO_DIRECT_DEFAULT 250 /* Arbitrary */ #define SMACK_CIPSO_MAPPED_DEFAULT 251 /* Also arbitrary */ -#define SMACK_CIPSO_MAXCATVAL 63 /* Bigger gets harder */ #define SMACK_CIPSO_MAXLEVEL 255 /* CIPSO 2.2 standard */ -#define SMACK_CIPSO_MAXCATNUM 239 /* CIPSO 2.2 standard */ +/* + * CIPSO 2.2 standard is 239, but Smack wants to use the + * categories in a structured way that limits the value to + * the bits in 23 bytes, hence the unusual number. + */ +#define SMACK_CIPSO_MAXCATNUM 184 /* 23 * 8 */ /* * Flag for transmute access diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index ce000a81caf7..19204e11c02c 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3066,6 +3066,8 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap, { struct smack_known *skp; int found = 0; + int acat; + int kcat; if ((sap->flags & NETLBL_SECATTR_MLS_LVL) != 0) { /* @@ -3082,12 +3084,28 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap, list_for_each_entry(skp, &smack_known_list, list) { if (sap->attr.mls.lvl != skp->smk_netlabel.attr.mls.lvl) continue; - if (memcmp(sap->attr.mls.cat, - skp->smk_netlabel.attr.mls.cat, - SMK_CIPSOLEN) != 0) - continue; - found = 1; - break; + /* + * Compare the catsets. Use the netlbl APIs. + */ + if ((sap->flags & NETLBL_SECATTR_MLS_CAT) == 0) { + if ((skp->smk_netlabel.flags & + NETLBL_SECATTR_MLS_CAT) == 0) + found = 1; + break; + } + for (acat = -1, kcat = -1; acat == kcat; ) { + acat = netlbl_secattr_catmap_walk( + sap->attr.mls.cat, acat + 1); + kcat = netlbl_secattr_catmap_walk( + skp->smk_netlabel.attr.mls.cat, + kcat + 1); + if (acat < 0 || kcat < 0) + break; + } + if (acat == kcat) { + found = 1; + break; + } } rcu_read_unlock(); diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index 269b270c6473..a07e93f00a0f 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -890,7 +890,7 @@ static ssize_t smk_set_cipso(struct file *file, const char __user *buf, for (i = 0; i < catlen; i++) { rule += SMK_DIGITLEN; ret = sscanf(rule, "%u", &cat); - if (ret != 1 || cat > SMACK_CIPSO_MAXCATVAL) + if (ret != 1 || cat > SMACK_CIPSO_MAXCATNUM) goto out; smack_catset_bit(cat, mapcatset); -- cgit v1.2.3 From 8af01f56a03e9cbd91a55d688fce1315021efba8 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 8 Aug 2013 20:11:22 -0400 Subject: cgroup: s/cgroup_subsys_state/cgroup_css/ s/task_subsys_state/task_css/ The names of the two struct cgroup_subsys_state accessors - cgroup_subsys_state() and task_subsys_state() - are somewhat awkward. The former clashes with the type name and the latter doesn't even indicate it's somehow related to cgroup. We're about to revamp large portion of cgroup API, so, let's rename them so that they're less awkward. Most per-controller usages of the accessors are localized in accessor wrappers and given the amount of scheduled changes, this isn't gonna add any noticeable headache. Rename cgroup_subsys_state() to cgroup_css() and task_subsys_state() to task_css(). This patch is pure rename. Signed-off-by: Tejun Heo Acked-by: Li Zefan --- security/device_cgroup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index e8aad69f0d69..87a0a037fbd6 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -58,12 +58,12 @@ static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s) static inline struct dev_cgroup *cgroup_to_devcgroup(struct cgroup *cgroup) { - return css_to_devcgroup(cgroup_subsys_state(cgroup, devices_subsys_id)); + return css_to_devcgroup(cgroup_css(cgroup, devices_subsys_id)); } static inline struct dev_cgroup *task_devcgroup(struct task_struct *task) { - return css_to_devcgroup(task_subsys_state(task, devices_subsys_id)); + return css_to_devcgroup(task_css(task, devices_subsys_id)); } struct cgroup_subsys devices_subsys; -- cgit v1.2.3 From a7c6d554aa01236ac2a9f851ab0f75704f76dfa2 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 8 Aug 2013 20:11:23 -0400 Subject: cgroup: add/update accessors which obtain subsys specific data from css css (cgroup_subsys_state) is usually embedded in a subsys specific data structure. Subsystems either use container_of() directly to cast from css to such data structure or has an accessor function wrapping such cast. As cgroup as whole is moving towards using css as the main interface handle, add and update such accessors to ease dealing with css's. All accessors explicitly handle NULL input and return NULL in those cases. While this looks like an extra branch in the code, as all controllers specific data structures have css as the first field, the casting doesn't involve any offsetting and the compiler can trivially optimize out the branch. * blkio, freezer, cpuset, cpu, cpuacct and net_cls didn't have such accessor. Added. * memory, hugetlb and devices already had one but didn't explicitly handle NULL input. Updated. Signed-off-by: Tejun Heo Acked-by: Li Zefan --- security/device_cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 87a0a037fbd6..90953648c643 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -53,7 +53,7 @@ struct dev_cgroup { static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s) { - return container_of(s, struct dev_cgroup, css); + return s ? container_of(s, struct dev_cgroup, css) : NULL; } static inline struct dev_cgroup *cgroup_to_devcgroup(struct cgroup *cgroup) -- cgit v1.2.3 From 6387698699afd72d6304566fb6ccf84bffe07c56 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 8 Aug 2013 20:11:23 -0400 Subject: cgroup: add css_parent() Currently, controllers have to explicitly follow the cgroup hierarchy to find the parent of a given css. cgroup is moving towards using cgroup_subsys_state as the main controller interface construct, so let's provide a way to climb the hierarchy using just csses. This patch implements css_parent() which, given a css, returns its parent. The function is guarnateed to valid non-NULL parent css as long as the target css is not at the top of the hierarchy. freezer, cpuset, cpu, cpuacct, hugetlb, memory, net_cls and devices are converted to use css_parent() instead of accessing cgroup->parent directly. * __parent_ca() is dropped from cpuacct and its usage is replaced with parent_ca(). The only difference between the two was NULL test on cgroup->parent which is now embedded in css_parent() making the distinction moot. Note that eventually a css->parent field will be added to css and the NULL check in css_parent() will go away. This patch shouldn't cause any behavior differences. Signed-off-by: Tejun Heo Acked-by: Li Zefan --- security/device_cgroup.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 90953648c643..635a49db005d 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -198,13 +198,11 @@ static inline bool is_devcg_online(const struct dev_cgroup *devcg) */ static int devcgroup_online(struct cgroup *cgroup) { - struct dev_cgroup *dev_cgroup, *parent_dev_cgroup = NULL; + struct dev_cgroup *dev_cgroup = cgroup_to_devcgroup(cgroup); + struct dev_cgroup *parent_dev_cgroup = css_to_devcgroup(css_parent(&dev_cgroup->css)); int ret = 0; mutex_lock(&devcgroup_mutex); - dev_cgroup = cgroup_to_devcgroup(cgroup); - if (cgroup->parent) - parent_dev_cgroup = cgroup_to_devcgroup(cgroup->parent); if (parent_dev_cgroup == NULL) dev_cgroup->behavior = DEVCG_DEFAULT_ALLOW; @@ -394,12 +392,10 @@ static bool may_access(struct dev_cgroup *dev_cgroup, static int parent_has_perm(struct dev_cgroup *childcg, struct dev_exception_item *ex) { - struct cgroup *pcg = childcg->css.cgroup->parent; - struct dev_cgroup *parent; + struct dev_cgroup *parent = css_to_devcgroup(css_parent(&childcg->css)); - if (!pcg) + if (!parent) return 1; - parent = cgroup_to_devcgroup(pcg); return may_access(parent, ex, childcg->behavior); } @@ -524,15 +520,11 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, char temp[12]; /* 11 + 1 characters needed for a u32 */ int count, rc = 0; struct dev_exception_item ex; - struct cgroup *p = devcgroup->css.cgroup; - struct dev_cgroup *parent = NULL; + struct dev_cgroup *parent = css_to_devcgroup(css_parent(&devcgroup->css)); if (!capable(CAP_SYS_ADMIN)) return -EPERM; - if (p->parent) - parent = cgroup_to_devcgroup(p->parent); - memset(&ex, 0, sizeof(ex)); b = buffer; -- cgit v1.2.3 From eb95419b023abacb415e2a18fea899023ce7624d Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 8 Aug 2013 20:11:23 -0400 Subject: cgroup: pass around cgroup_subsys_state instead of cgroup in subsystem methods cgroup is currently in the process of transitioning to using struct cgroup_subsys_state * as the primary handle instead of struct cgroup * in subsystem implementations for the following reasons. * With unified hierarchy, subsystems will be dynamically bound and unbound from cgroups and thus css's (cgroup_subsys_state) may be created and destroyed dynamically over the lifetime of a cgroup, which is different from the current state where all css's are allocated and destroyed together with the associated cgroup. This in turn means that cgroup_css() should be synchronized and may return NULL, making it more cumbersome to use. * Differing levels of per-subsystem granularity in the unified hierarchy means that the task and descendant iterators should behave differently depending on the specific subsystem the iteration is being performed for. * In majority of the cases, subsystems only care about its part in the cgroup hierarchy - ie. the hierarchy of css's. Subsystem methods often obtain the matching css pointer from the cgroup and don't bother with the cgroup pointer itself. Passing around css fits much better. This patch converts all cgroup_subsys methods to take @css instead of @cgroup. The conversions are mostly straight-forward. A few noteworthy changes are * ->css_alloc() now takes css of the parent cgroup rather than the pointer to the new cgroup as the css for the new cgroup doesn't exist yet. Knowing the parent css is enough for all the existing subsystems. * In kernel/cgroup.c::offline_css(), unnecessary open coded css dereference is replaced with local variable access. This patch shouldn't cause any behavior differences. v2: Unnecessary explicit cgrp->subsys[] deref in css_online() replaced with local variable @css as suggested by Li Zefan. Rebased on top of new for-3.12 which includes for-3.11-fixes so that ->css_free() invocation added by da0a12caff ("cgroup: fix a leak when percpu_ref_init() fails") is converted too. Suggested by Li Zefan. Signed-off-by: Tejun Heo Acked-by: Li Zefan Acked-by: Michal Hocko Acked-by: Vivek Goyal Acked-by: Aristeu Rozanski Acked-by: Daniel Wagner Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Johannes Weiner Cc: Balbir Singh Cc: Matt Helsley Cc: Jens Axboe Cc: Steven Rostedt --- security/device_cgroup.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 635a49db005d..7293ac49ba7b 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -68,7 +68,7 @@ static inline struct dev_cgroup *task_devcgroup(struct task_struct *task) struct cgroup_subsys devices_subsys; -static int devcgroup_can_attach(struct cgroup *new_cgrp, +static int devcgroup_can_attach(struct cgroup_subsys_state *new_css, struct cgroup_taskset *set) { struct task_struct *task = cgroup_taskset_first(set); @@ -193,13 +193,13 @@ static inline bool is_devcg_online(const struct dev_cgroup *devcg) /** * devcgroup_online - initializes devcgroup's behavior and exceptions based on * parent's - * @cgroup: cgroup getting online + * @css: css getting online * returns 0 in case of success, error code otherwise */ -static int devcgroup_online(struct cgroup *cgroup) +static int devcgroup_online(struct cgroup_subsys_state *css) { - struct dev_cgroup *dev_cgroup = cgroup_to_devcgroup(cgroup); - struct dev_cgroup *parent_dev_cgroup = css_to_devcgroup(css_parent(&dev_cgroup->css)); + struct dev_cgroup *dev_cgroup = css_to_devcgroup(css); + struct dev_cgroup *parent_dev_cgroup = css_to_devcgroup(css_parent(css)); int ret = 0; mutex_lock(&devcgroup_mutex); @@ -217,9 +217,9 @@ static int devcgroup_online(struct cgroup *cgroup) return ret; } -static void devcgroup_offline(struct cgroup *cgroup) +static void devcgroup_offline(struct cgroup_subsys_state *css) { - struct dev_cgroup *dev_cgroup = cgroup_to_devcgroup(cgroup); + struct dev_cgroup *dev_cgroup = css_to_devcgroup(css); mutex_lock(&devcgroup_mutex); dev_cgroup->behavior = DEVCG_DEFAULT_NONE; @@ -229,7 +229,8 @@ static void devcgroup_offline(struct cgroup *cgroup) /* * called from kernel/cgroup.c with cgroup_lock() held. */ -static struct cgroup_subsys_state *devcgroup_css_alloc(struct cgroup *cgroup) +static struct cgroup_subsys_state * +devcgroup_css_alloc(struct cgroup_subsys_state *parent_css) { struct dev_cgroup *dev_cgroup; @@ -242,11 +243,10 @@ static struct cgroup_subsys_state *devcgroup_css_alloc(struct cgroup *cgroup) return &dev_cgroup->css; } -static void devcgroup_css_free(struct cgroup *cgroup) +static void devcgroup_css_free(struct cgroup_subsys_state *css) { - struct dev_cgroup *dev_cgroup; + struct dev_cgroup *dev_cgroup = css_to_devcgroup(css); - dev_cgroup = cgroup_to_devcgroup(cgroup); __dev_exception_clean(dev_cgroup); kfree(dev_cgroup); } -- cgit v1.2.3 From 182446d087906de40e514573a92a97b203695f71 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 8 Aug 2013 20:11:24 -0400 Subject: cgroup: pass around cgroup_subsys_state instead of cgroup in file methods cgroup is currently in the process of transitioning to using struct cgroup_subsys_state * as the primary handle instead of struct cgroup. Please see the previous commit which converts the subsystem methods for rationale. This patch converts all cftype file operations to take @css instead of @cgroup. cftypes for the cgroup core files don't have their subsytem pointer set. These will automatically use the dummy_css added by the previous patch and can be converted the same way. Most subsystem conversions are straight forwards but there are some interesting ones. * freezer: update_if_frozen() is also converted to take @css instead of @cgroup for consistency. This will make the code look simpler too once iterators are converted to use css. * memory/vmpressure: mem_cgroup_from_css() needs to be exported to vmpressure while mem_cgroup_from_cont() can be made static. Updated accordingly. * cpu: cgroup_tg() doesn't have any user left. Removed. * cpuacct: cgroup_ca() doesn't have any user left. Removed. * hugetlb: hugetlb_cgroup_form_cgroup() doesn't have any user left. Removed. * net_cls: cgrp_cls_state() doesn't have any user left. Removed. Signed-off-by: Tejun Heo Acked-by: Li Zefan Acked-by: Michal Hocko Acked-by: Vivek Goyal Acked-by: Aristeu Rozanski Acked-by: Daniel Wagner Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Johannes Weiner Cc: Balbir Singh Cc: Matt Helsley Cc: Jens Axboe Cc: Steven Rostedt --- security/device_cgroup.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 7293ac49ba7b..e0ca464fa854 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -289,10 +289,10 @@ static void set_majmin(char *str, unsigned m) sprintf(str, "%u", m); } -static int devcgroup_seq_read(struct cgroup *cgroup, struct cftype *cft, - struct seq_file *m) +static int devcgroup_seq_read(struct cgroup_subsys_state *css, + struct cftype *cft, struct seq_file *m) { - struct dev_cgroup *devcgroup = cgroup_to_devcgroup(cgroup); + struct dev_cgroup *devcgroup = css_to_devcgroup(css); struct dev_exception_item *ex; char maj[MAJMINLEN], min[MAJMINLEN], acc[ACCLEN]; @@ -669,13 +669,13 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, return rc; } -static int devcgroup_access_write(struct cgroup *cgrp, struct cftype *cft, - const char *buffer) +static int devcgroup_access_write(struct cgroup_subsys_state *css, + struct cftype *cft, const char *buffer) { int retval; mutex_lock(&devcgroup_mutex); - retval = devcgroup_update_access(cgroup_to_devcgroup(cgrp), + retval = devcgroup_update_access(css_to_devcgroup(css), cft->private, buffer); mutex_unlock(&devcgroup_mutex); return retval; -- cgit v1.2.3 From 492eb21b98f88e411a8bb43d6edcd7d7022add10 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 8 Aug 2013 20:11:25 -0400 Subject: cgroup: make hierarchy iterators deal with cgroup_subsys_state instead of cgroup cgroup is currently in the process of transitioning to using css (cgroup_subsys_state) as the primary handle instead of cgroup in subsystem API. For hierarchy iterators, this is beneficial because * In most cases, css is the only thing subsystems care about anyway. * On the planned unified hierarchy, iterations for different subsystems will need to skip over different subtrees of the hierarchy depending on which subsystems are enabled on each cgroup. Passing around css makes it unnecessary to explicitly specify the subsystem in question as css is intersection between cgroup and subsystem * For the planned unified hierarchy, css's would need to be created and destroyed dynamically independent from cgroup hierarchy. Having cgroup core manage css iteration makes enforcing deref rules a lot easier. Most subsystem conversions are straight-forward. Noteworthy changes are * blkio: cgroup_to_blkcg() is no longer used. Removed. * freezer: cgroup_freezer() is no longer used. Removed. * devices: cgroup_to_devcgroup() is no longer used. Removed. Signed-off-by: Tejun Heo Acked-by: Li Zefan Acked-by: Michal Hocko Acked-by: Vivek Goyal Acked-by: Aristeu Rozanski Cc: Johannes Weiner Cc: Balbir Singh Cc: Matt Helsley Cc: Jens Axboe --- security/device_cgroup.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index e0ca464fa854..9bf230aa28b0 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -56,11 +56,6 @@ static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s) return s ? container_of(s, struct dev_cgroup, css) : NULL; } -static inline struct dev_cgroup *cgroup_to_devcgroup(struct cgroup *cgroup) -{ - return css_to_devcgroup(cgroup_css(cgroup, devices_subsys_id)); -} - static inline struct dev_cgroup *task_devcgroup(struct task_struct *task) { return css_to_devcgroup(task_css(task, devices_subsys_id)); @@ -447,13 +442,13 @@ static void revalidate_active_exceptions(struct dev_cgroup *devcg) static int propagate_exception(struct dev_cgroup *devcg_root, struct dev_exception_item *ex) { - struct cgroup *root = devcg_root->css.cgroup, *pos; + struct cgroup_subsys_state *pos; int rc = 0; rcu_read_lock(); - cgroup_for_each_descendant_pre(pos, root) { - struct dev_cgroup *devcg = cgroup_to_devcgroup(pos); + css_for_each_descendant_pre(pos, &devcg_root->css) { + struct dev_cgroup *devcg = css_to_devcgroup(pos); /* * Because devcgroup_mutex is held, no devcg will become -- cgit v1.2.3 From bd8815a6d802fc16a7a106e170593aa05dc17e72 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 8 Aug 2013 20:11:27 -0400 Subject: cgroup: make css_for_each_descendant() and friends include the origin css in the iteration Previously, all css descendant iterators didn't include the origin (root of subtree) css in the iteration. The reasons were maintaining consistency with css_for_each_child() and that at the time of introduction more use cases needed skipping the origin anyway; however, given that css_is_descendant() considers self to be a descendant, omitting the origin css has become more confusing and looking at the accumulated use cases rather clearly indicates that including origin would result in simpler code overall. While this is a change which can easily lead to subtle bugs, cgroup API including the iterators has recently gone through major restructuring and no out-of-tree changes will be applicable without adjustments making this a relatively acceptable opportunity for this type of change. The conversions are mostly straight-forward. If the iteration block had explicit origin handling before or after, it's moved inside the iteration. If not, if (pos == origin) continue; is added. Some conversions add extra reference get/put around origin handling by consolidating origin handling and the rest. While the extra ref operations aren't strictly necessary, this shouldn't cause any noticeable difference. Signed-off-by: Tejun Heo Acked-by: Li Zefan Acked-by: Vivek Goyal Acked-by: Aristeu Rozanski Acked-by: Michal Hocko Cc: Jens Axboe Cc: Matt Helsley Cc: Johannes Weiner Cc: Balbir Singh --- security/device_cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 9bf230aa28b0..c123628d3f84 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -456,7 +456,7 @@ static int propagate_exception(struct dev_cgroup *devcg_root, * methods), and online ones are safe to access outside RCU * read lock without bumping refcnt. */ - if (!is_devcg_online(devcg)) + if (pos == &devcg_root->css || !is_devcg_online(devcg)) continue; rcu_read_unlock(); -- cgit v1.2.3 From 10289b0f738e8b301969f2288c4942455f1b1e59 Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Fri, 9 Aug 2013 11:47:07 +0200 Subject: Smack: parse multiple rules per write to load2, up to PAGE_SIZE-1 bytes Smack interface for loading rules has always parsed only single rule from data written to it. This requires user program to call one write() per each rule it wants to load. This change makes it possible to write multiple rules, separated by new line character. Smack will load at most PAGE_SIZE-1 characters and properly return number of processed bytes. In case when user buffer is larger, it will be additionally truncated. All characters after last \n will not get parsed to avoid partial rule near input buffer boundary. Signed-off-by: Rafal Krypa --- security/smack/smackfs.c | 167 +++++++++++++++++++++++------------------------ 1 file changed, 82 insertions(+), 85 deletions(-) (limited to 'security') diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index a07e93f00a0f..80f4b4a45725 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -368,56 +368,43 @@ static int smk_parse_rule(const char *data, struct smack_parsed_rule *rule, * @data: string to be parsed, null terminated * @rule: Will be filled with Smack parsed rule * @import: if non-zero, import labels - * @change: if non-zero, data is from /smack/change-rule + * @tokens: numer of substrings expected in data * - * Returns 0 on success, -1 on failure + * Returns number of processed bytes on success, -1 on failure. */ -static int smk_parse_long_rule(const char *data, struct smack_parsed_rule *rule, - int import, int change) +static ssize_t smk_parse_long_rule(char *data, struct smack_parsed_rule *rule, + int import, int tokens) { - char *subject; - char *object; - char *access1; - char *access2; - int datalen; - int rc = -1; + ssize_t cnt = 0; + char *tok[4]; + int i; - /* This is inefficient */ - datalen = strlen(data); + /* + * Parsing the rule in-place, filling all white-spaces with '\0' + */ + for (i = 0; i < tokens; ++i) { + while (isspace(data[cnt])) + data[cnt++] = '\0'; - /* Our first element can be 64 + \0 with no spaces */ - subject = kzalloc(datalen + 1, GFP_KERNEL); - if (subject == NULL) - return -1; - object = kzalloc(datalen, GFP_KERNEL); - if (object == NULL) - goto free_out_s; - access1 = kzalloc(datalen, GFP_KERNEL); - if (access1 == NULL) - goto free_out_o; - access2 = kzalloc(datalen, GFP_KERNEL); - if (access2 == NULL) - goto free_out_a; - - if (change) { - if (sscanf(data, "%s %s %s %s", - subject, object, access1, access2) == 4) - rc = smk_fill_rule(subject, object, access1, access2, - rule, import, 0); - } else { - if (sscanf(data, "%s %s %s", subject, object, access1) == 3) - rc = smk_fill_rule(subject, object, access1, NULL, - rule, import, 0); + if (data[cnt] == '\0') + /* Unexpected end of data */ + return -1; + + tok[i] = data + cnt; + + while (data[cnt] && !isspace(data[cnt])) + ++cnt; } + while (isspace(data[cnt])) + data[cnt++] = '\0'; - kfree(access2); -free_out_a: - kfree(access1); -free_out_o: - kfree(object); -free_out_s: - kfree(subject); - return rc; + while (i < 4) + tok[i++] = NULL; + + if (smk_fill_rule(tok[0], tok[1], tok[2], tok[3], rule, import, 0)) + return -1; + + return cnt; } #define SMK_FIXED24_FMT 0 /* Fixed 24byte label format */ @@ -449,9 +436,10 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf, { struct smack_parsed_rule rule; char *data; - int datalen; - int rc = -EINVAL; - int load = 0; + int rc; + int trunc = 0; + int tokens; + ssize_t cnt = 0; /* * No partial writes. @@ -466,11 +454,14 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf, */ if (count != SMK_OLOADLEN && count != SMK_LOADLEN) return -EINVAL; - datalen = SMK_LOADLEN; - } else - datalen = count + 1; + } else { + if (count >= PAGE_SIZE) { + count = PAGE_SIZE - 1; + trunc = 1; + } + } - data = kzalloc(datalen, GFP_KERNEL); + data = kmalloc(count + 1, GFP_KERNEL); if (data == NULL) return -ENOMEM; @@ -479,36 +470,49 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf, goto out; } - if (format == SMK_LONG_FMT) { - /* - * Be sure the data string is terminated. - */ - data[count] = '\0'; - if (smk_parse_long_rule(data, &rule, 1, 0)) - goto out; - } else if (format == SMK_CHANGE_FMT) { - data[count] = '\0'; - if (smk_parse_long_rule(data, &rule, 1, 1)) - goto out; - } else { - /* - * More on the minor hack for backward compatibility - */ - if (count == (SMK_OLOADLEN)) - data[SMK_OLOADLEN] = '-'; - if (smk_parse_rule(data, &rule, 1)) + /* + * In case of parsing only part of user buf, + * avoid having partial rule at the data buffer + */ + if (trunc) { + while (count > 0 && (data[count - 1] != '\n')) + --count; + if (count == 0) { + rc = -EINVAL; goto out; + } } - if (rule_list == NULL) { - load = 1; - rule_list = &rule.smk_subject->smk_rules; - rule_lock = &rule.smk_subject->smk_rules_lock; + data[count] = '\0'; + tokens = (format == SMK_CHANGE_FMT ? 4 : 3); + while (cnt < count) { + if (format == SMK_FIXED24_FMT) { + rc = smk_parse_rule(data, &rule, 1); + if (rc != 0) { + rc = -EINVAL; + goto out; + } + cnt = count; + } else { + rc = smk_parse_long_rule(data + cnt, &rule, 1, tokens); + if (rc <= 0) { + rc = -EINVAL; + goto out; + } + cnt += rc; + } + + if (rule_list == NULL) + rc = smk_set_access(&rule, &rule.smk_subject->smk_rules, + &rule.smk_subject->smk_rules_lock, 1); + else + rc = smk_set_access(&rule, rule_list, rule_lock, 0); + + if (rc) + goto out; } - rc = smk_set_access(&rule, rule_list, rule_lock, load); - if (rc == 0) - rc = count; + rc = cnt; out: kfree(data); return rc; @@ -1829,7 +1833,6 @@ static ssize_t smk_user_access(struct file *file, const char __user *buf, { struct smack_parsed_rule rule; char *data; - char *cod; int res; data = simple_transaction_get(file, buf, count); @@ -1842,18 +1845,12 @@ static ssize_t smk_user_access(struct file *file, const char __user *buf, res = smk_parse_rule(data, &rule, 0); } else { /* - * Copy the data to make sure the string is terminated. + * simple_transaction_get() returns null-terminated data */ - cod = kzalloc(count + 1, GFP_KERNEL); - if (cod == NULL) - return -ENOMEM; - memcpy(cod, data, count); - cod[count] = '\0'; - res = smk_parse_long_rule(cod, &rule, 0, 0); - kfree(cod); + res = smk_parse_long_rule(data, &rule, 0, 3); } - if (res) + if (res < 0) return -EINVAL; res = smk_access(rule.smk_subject, rule.smk_object, -- cgit v1.2.3 From dfe4ac28be73833556756dca6771d4274a7f1157 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 17 Jun 2013 21:25:08 +0900 Subject: apparmor: remove minimum size check for vmalloc() 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 Signed-off-by: John Johansen --- security/apparmor/lib.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'security') diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index fcfe0233574c..69689922c491 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -97,11 +97,6 @@ void *__aa_kvmalloc(size_t size, gfp_t flags) if (size <= (16*PAGE_SIZE)) buffer = kmalloc(size, flags | GFP_NOIO | __GFP_NOWARN); if (!buffer) { - /* see kvfree for why size must be at least work_struct size - * when allocated via vmalloc - */ - if (size < sizeof(struct work_struct)) - size = sizeof(struct work_struct); if (flags & __GFP_ZERO) buffer = vzalloc(size); else -- cgit v1.2.3 From c611616cd3cb27f9605ee4954532b3fe144d951b Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:03:43 -0700 Subject: apparmor: enable users to query whether apparmor is enabled Signed-off-by: John Johansen --- security/apparmor/lsm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 2e2a0dd4a73f..96506dfe51ec 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -742,7 +742,7 @@ module_param_named(paranoid_load, aa_g_paranoid_load, aabool, /* Boot time disable flag */ static bool apparmor_enabled = CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE; -module_param_named(enabled, apparmor_enabled, aabool, S_IRUSR); +module_param_named(enabled, apparmor_enabled, bool, S_IRUGO); static int __init apparmor_enabled_setup(char *str) { -- cgit v1.2.3 From 9d910a3bc01008d432b3bb79a69e7e3cdb4821b2 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:04:43 -0700 Subject: apparmor: add a features/policy dir to interface 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 Acked-by: Kees Cook --- security/apparmor/apparmorfs.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'security') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 16c15ec6f670..ad6c74892b5f 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -198,7 +198,12 @@ static struct aa_fs_entry aa_fs_entry_domain[] = { { } }; +static struct aa_fs_entry aa_fs_entry_policy[] = { + {} +}; + static struct aa_fs_entry aa_fs_entry_features[] = { + AA_FS_DIR("policy", aa_fs_entry_policy), AA_FS_DIR("domain", aa_fs_entry_domain), AA_FS_DIR("file", aa_fs_entry_file), AA_FS_FILE_U64("capability", VFS_CAP_FLAGS_MASK), -- cgit v1.2.3 From dd51c84857630e77c139afe4d9bba65fc051dc3f Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:05:43 -0700 Subject: apparmor: provide base for multiple profiles to be replaced at once 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 --- security/apparmor/apparmorfs.c | 1 + security/apparmor/include/policy_unpack.h | 14 +- security/apparmor/policy.c | 300 ++++++++++++++++++------------ security/apparmor/policy_unpack.c | 114 +++++++++--- 4 files changed, 283 insertions(+), 146 deletions(-) (limited to 'security') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index ad6c74892b5f..3ed56e21a9fd 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -199,6 +199,7 @@ static struct aa_fs_entry aa_fs_entry_domain[] = { }; static struct aa_fs_entry aa_fs_entry_policy[] = { + AA_FS_FILE_BOOLEAN("set_load", 1), {} }; diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h index a2dcccac45aa..0d7ad722b8ff 100644 --- a/security/apparmor/include/policy_unpack.h +++ b/security/apparmor/include/policy_unpack.h @@ -15,6 +15,18 @@ #ifndef __POLICY_INTERFACE_H #define __POLICY_INTERFACE_H -struct aa_profile *aa_unpack(void *udata, size_t size, const char **ns); +#include + +struct aa_load_ent { + struct list_head list; + struct aa_profile *new; + struct aa_profile *old; + struct aa_profile *rename; +}; + +void aa_load_ent_free(struct aa_load_ent *ent); +struct aa_load_ent *aa_load_ent_alloc(void); + +int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns); #endif /* __POLICY_INTERFACE_H */ diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 0f345c4dee5f..407b442c0a2c 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -472,45 +472,6 @@ static void __list_remove_profile(struct aa_profile *profile) aa_put_profile(profile); } -/** - * __replace_profile - replace @old with @new on a list - * @old: profile to be replaced (NOT NULL) - * @new: profile to replace @old with (NOT NULL) - * - * Will duplicate and refcount elements that @new inherits from @old - * and will inherit @old children. - * - * refcount @new for list, put @old list refcount - * - * Requires: namespace list lock be held, or list not be shared - */ -static void __replace_profile(struct aa_profile *old, struct aa_profile *new) -{ - struct aa_policy *policy; - struct aa_profile *child, *tmp; - - if (old->parent) - policy = &old->parent->base; - else - policy = &old->ns->base; - - /* released when @new is freed */ - new->parent = aa_get_profile(old->parent); - new->ns = aa_get_namespace(old->ns); - __list_add_profile(&policy->profiles, new); - /* inherit children */ - list_for_each_entry_safe(child, tmp, &old->base.profiles, base.list) { - aa_put_profile(child->parent); - child->parent = aa_get_profile(new); - /* list refcount transferred to @new*/ - list_move(&child->base.list, &new->base.profiles); - } - - /* released by free_profile */ - old->replacedby = aa_get_profile(new); - __list_remove_profile(old); -} - static void __profile_list_release(struct list_head *head); /** @@ -952,25 +913,6 @@ static int replacement_allowed(struct aa_profile *profile, int noreplace, return 0; } -/** - * __add_new_profile - simple wrapper around __list_add_profile - * @ns: namespace that profile is being added to (NOT NULL) - * @policy: the policy container to add the profile to (NOT NULL) - * @profile: profile to add (NOT NULL) - * - * add a profile to a list and do other required basic allocations - */ -static void __add_new_profile(struct aa_namespace *ns, struct aa_policy *policy, - struct aa_profile *profile) -{ - if (policy != &ns->base) - /* released on profile replacement or free_profile */ - profile->parent = aa_get_profile((struct aa_profile *) policy); - __list_add_profile(&policy->profiles, profile); - /* released on free_profile */ - profile->ns = aa_get_namespace(ns); -} - /** * aa_audit_policy - Do auditing of policy changes * @op: policy operation being performed @@ -1019,6 +961,109 @@ bool aa_may_manage_policy(int op) return 1; } +static struct aa_profile *__list_lookup_parent(struct list_head *lh, + struct aa_profile *profile) +{ + const char *base = hname_tail(profile->base.hname); + long len = base - profile->base.hname; + struct aa_load_ent *ent; + + /* parent won't have trailing // so remove from len */ + if (len <= 2) + return NULL; + len -= 2; + + list_for_each_entry(ent, lh, list) { + if (ent->new == profile) + continue; + if (strncmp(ent->new->base.hname, profile->base.hname, len) == + 0 && ent->new->base.hname[len] == 0) + return ent->new; + } + + return NULL; +} + +/** + * __replace_profile - replace @old with @new on a list + * @old: profile to be replaced (NOT NULL) + * @new: profile to replace @old with (NOT NULL) + * + * Will duplicate and refcount elements that @new inherits from @old + * and will inherit @old children. + * + * refcount @new for list, put @old list refcount + * + * Requires: namespace list lock be held, or list not be shared + */ +static void __replace_profile(struct aa_profile *old, struct aa_profile *new) +{ + struct aa_profile *child, *tmp; + + if (!list_empty(&old->base.profiles)) { + LIST_HEAD(lh); + list_splice_init(&old->base.profiles, &lh); + + list_for_each_entry_safe(child, tmp, &lh, base.list) { + struct aa_profile *p; + + list_del_init(&child->base.list); + p = __find_child(&new->base.profiles, child->base.name); + if (p) { + /* @p replaces @child */ + __replace_profile(child, p); + continue; + } + + /* inherit @child and its children */ + /* TODO: update hname of inherited children */ + /* list refcount transferred to @new */ + list_add(&child->base.list, &new->base.profiles); + aa_put_profile(child->parent); + child->parent = aa_get_profile(new); + } + } + + if (!new->parent) + new->parent = aa_get_profile(old->parent); + /* released by free_profile */ + old->replacedby = aa_get_profile(new); + + if (list_empty(&new->base.list)) { + /* new is not on a list already */ + list_replace_init(&old->base.list, &new->base.list); + aa_get_profile(new); + aa_put_profile(old); + } else + __list_remove_profile(old); +} + +/** + * __lookup_replace - lookup replacement information for a profile + * @ns - namespace the lookup occurs in + * @hname - name of profile to lookup + * @noreplace - true if not replacing an existing profile + * @p - Returns: profile to be replaced + * @info - Returns: info string on why lookup failed + * + * Returns: profile to replace (no ref) on success else ptr error + */ +static int __lookup_replace(struct aa_namespace *ns, const char *hname, + bool noreplace, struct aa_profile **p, + const char **info) +{ + *p = aa_get_profile(__lookup_profile(&ns->base, hname)); + if (*p) { + int error = replacement_allowed(*p, noreplace, info); + if (error) { + *info = "profile can not be replaced"; + return error; + } + } + + return 0; +} + /** * aa_replace_profiles - replace profile(s) on the profile list * @udata: serialized data stream (NOT NULL) @@ -1033,21 +1078,17 @@ bool aa_may_manage_policy(int op) */ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) { - struct aa_policy *policy; - struct aa_profile *old_profile = NULL, *new_profile = NULL; - struct aa_profile *rename_profile = NULL; - struct aa_namespace *ns = NULL; const char *ns_name, *name = NULL, *info = NULL; + struct aa_namespace *ns = NULL; + struct aa_load_ent *ent, *tmp; int op = OP_PROF_REPL; ssize_t error; + LIST_HEAD(lh); /* released below */ - new_profile = aa_unpack(udata, size, &ns_name); - if (IS_ERR(new_profile)) { - error = PTR_ERR(new_profile); - new_profile = NULL; - goto fail; - } + error = aa_unpack(udata, size, &lh, &ns_name); + if (error) + goto out; /* released below */ ns = aa_prepare_namespace(ns_name); @@ -1058,71 +1099,96 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) goto fail; } - name = new_profile->base.hname; - write_lock(&ns->lock); - /* no ref on policy only use inside lock */ - policy = __lookup_parent(ns, new_profile->base.hname); - - if (!policy) { - info = "parent does not exist"; - error = -ENOENT; - goto audit; - } - - old_profile = __find_child(&policy->profiles, new_profile->base.name); - /* released below */ - aa_get_profile(old_profile); - - if (new_profile->rename) { - rename_profile = __lookup_profile(&ns->base, - new_profile->rename); - /* released below */ - aa_get_profile(rename_profile); - - if (!rename_profile) { - info = "profile to rename does not exist"; - name = new_profile->rename; - error = -ENOENT; - goto audit; + /* setup parent and ns info */ + list_for_each_entry(ent, &lh, list) { + struct aa_policy *policy; + + name = ent->new->base.hname; + error = __lookup_replace(ns, ent->new->base.hname, noreplace, + &ent->old, &info); + if (error) + goto fail_lock; + + if (ent->new->rename) { + error = __lookup_replace(ns, ent->new->rename, + noreplace, &ent->rename, + &info); + if (error) + goto fail_lock; } - } - - error = replacement_allowed(old_profile, noreplace, &info); - if (error) - goto audit; - error = replacement_allowed(rename_profile, noreplace, &info); - if (error) - goto audit; - -audit: - if (!old_profile && !rename_profile) - op = OP_PROF_LOAD; + /* released when @new is freed */ + ent->new->ns = aa_get_namespace(ns); + + if (ent->old || ent->rename) + continue; + + /* no ref on policy only use inside lock */ + policy = __lookup_parent(ns, ent->new->base.hname); + if (!policy) { + struct aa_profile *p; + p = __list_lookup_parent(&lh, ent->new); + if (!p) { + error = -ENOENT; + info = "parent does not exist"; + name = ent->new->base.hname; + goto fail_lock; + } + ent->new->parent = aa_get_profile(p); + } else if (policy != &ns->base) + /* released on profile replacement or free_profile */ + ent->new->parent = aa_get_profile((struct aa_profile *) + policy); + } - error = audit_policy(op, GFP_ATOMIC, name, info, error); + /* do actual replacement */ + list_for_each_entry_safe(ent, tmp, &lh, list) { + list_del_init(&ent->list); + op = (!ent->old && !ent->rename) ? OP_PROF_LOAD : OP_PROF_REPL; + + audit_policy(op, GFP_ATOMIC, ent->new->base.name, NULL, error); + + if (ent->old) { + __replace_profile(ent->old, ent->new); + if (ent->rename) + __replace_profile(ent->rename, ent->new); + } else if (ent->rename) { + __replace_profile(ent->rename, ent->new); + } else if (ent->new->parent) { + struct aa_profile *parent; + parent = aa_newest_version(ent->new->parent); + /* parent replaced in this atomic set? */ + if (parent != ent->new->parent) { + aa_get_profile(parent); + aa_put_profile(ent->new->parent); + ent->new->parent = parent; + } + __list_add_profile(&parent->base.profiles, ent->new); + } else + __list_add_profile(&ns->base.profiles, ent->new); - if (!error) { - if (rename_profile) - __replace_profile(rename_profile, new_profile); - if (old_profile) - __replace_profile(old_profile, new_profile); - if (!(old_profile || rename_profile)) - __add_new_profile(ns, policy, new_profile); + aa_load_ent_free(ent); } write_unlock(&ns->lock); out: aa_put_namespace(ns); - aa_put_profile(rename_profile); - aa_put_profile(old_profile); - aa_put_profile(new_profile); + if (error) return error; return size; +fail_lock: + write_unlock(&ns->lock); fail: error = audit_policy(op, GFP_KERNEL, name, info, error); + + list_for_each_entry_safe(ent, tmp, &lh, list) { + list_del_init(&ent->list); + aa_load_ent_free(ent); + } + goto out; } diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 6dac7d77cb4d..080a26b11f01 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -333,8 +333,10 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e) /* * The dfa is aligned with in the blob to 8 bytes * from the beginning of the stream. + * alignment adjust needed by dfa unpack */ - size_t sz = blob - (char *)e->start; + size_t sz = blob - (char *) e->start - + ((e->pos - e->start) & 7); size_t pad = ALIGN(sz, 8) - sz; int flags = TO_ACCEPT1_FLAG(YYTD_DATA32) | TO_ACCEPT2_FLAG(YYTD_DATA32); @@ -622,29 +624,41 @@ fail: /** * verify_head - unpack serialized stream header * @e: serialized data read head (NOT NULL) + * @required: whether the header is required or optional * @ns: Returns - namespace if one is specified else NULL (NOT NULL) * * Returns: error or 0 if header is good */ -static int verify_header(struct aa_ext *e, const char **ns) +static int verify_header(struct aa_ext *e, int required, const char **ns) { int error = -EPROTONOSUPPORT; + const char *name = NULL; + *ns = NULL; + /* get the interface version */ if (!unpack_u32(e, &e->version, "version")) { - audit_iface(NULL, NULL, "invalid profile format", e, error); - return error; - } + if (required) { + audit_iface(NULL, NULL, "invalid profile format", e, + error); + return error; + } - /* check that the interface version is currently supported */ - if (e->version != 5) { - audit_iface(NULL, NULL, "unsupported interface version", e, - error); - return error; + /* check that the interface version is currently supported */ + if (e->version != 5) { + audit_iface(NULL, NULL, "unsupported interface version", + e, error); + return error; + } } + /* read the namespace if present */ - if (!unpack_str(e, ns, "namespace")) - *ns = NULL; + if (unpack_str(e, &name, "namespace")) { + if (*ns && strcmp(*ns, name)) + audit_iface(NULL, NULL, "invalid ns change", e, error); + else if (!*ns) + *ns = name; + } return 0; } @@ -693,18 +707,40 @@ static int verify_profile(struct aa_profile *profile) return 0; } +void aa_load_ent_free(struct aa_load_ent *ent) +{ + if (ent) { + aa_put_profile(ent->rename); + aa_put_profile(ent->old); + aa_put_profile(ent->new); + kzfree(ent); + } +} + +struct aa_load_ent *aa_load_ent_alloc(void) +{ + struct aa_load_ent *ent = kzalloc(sizeof(*ent), GFP_KERNEL); + if (ent) + INIT_LIST_HEAD(&ent->list); + return ent; +} + /** - * aa_unpack - unpack packed binary profile data loaded from user space + * aa_unpack - unpack packed binary profile(s) data loaded from user space * @udata: user data copied to kmem (NOT NULL) * @size: the size of the user data + * @lh: list to place unpacked profiles in a aa_repl_ws * @ns: Returns namespace profile is in if specified else NULL (NOT NULL) * - * Unpack user data and return refcounted allocated profile or ERR_PTR + * Unpack user data and return refcounted allocated profile(s) stored in + * @lh in order of discovery, with the list chain stored in base.list + * or error * - * Returns: profile else error pointer if fails to unpack + * Returns: profile(s) on @lh else error pointer if fails to unpack */ -struct aa_profile *aa_unpack(void *udata, size_t size, const char **ns) +int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns) { + struct aa_load_ent *tmp, *ent; struct aa_profile *profile = NULL; int error; struct aa_ext e = { @@ -713,20 +749,42 @@ struct aa_profile *aa_unpack(void *udata, size_t size, const char **ns) .pos = udata, }; - error = verify_header(&e, ns); - if (error) - return ERR_PTR(error); + *ns = NULL; + while (e.pos < e.end) { + error = verify_header(&e, e.pos == e.start, ns); + if (error) + goto fail; - profile = unpack_profile(&e); - if (IS_ERR(profile)) - return profile; + profile = unpack_profile(&e); + if (IS_ERR(profile)) { + error = PTR_ERR(profile); + goto fail; + } + + error = verify_profile(profile); + if (error) { + aa_put_profile(profile); + goto fail; + } + + ent = aa_load_ent_alloc(); + if (!ent) { + error = -ENOMEM; + aa_put_profile(profile); + goto fail; + } - error = verify_profile(profile); - if (error) { - aa_put_profile(profile); - profile = ERR_PTR(error); + ent->new = profile; + list_add_tail(&ent->list, lh); } - /* return refcount */ - return profile; + return 0; + +fail: + list_for_each_entry_safe(ent, tmp, lh, list) { + list_del_init(&ent->list); + aa_load_ent_free(ent); + } + + return error; } -- cgit v1.2.3 From 01e2b670aa898a39259bc85c78e3d74820f4d3b6 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:06:43 -0700 Subject: apparmor: convert profile lists to RCU based locking Signed-off-by: John Johansen --- security/apparmor/domain.c | 14 ++- security/apparmor/include/apparmor.h | 6 + security/apparmor/include/policy.h | 45 +++++++- security/apparmor/policy.c | 213 ++++++++++++++++++----------------- 4 files changed, 167 insertions(+), 111 deletions(-) (limited to 'security') diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 01b7bd669a88..454bcd7f3452 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -144,7 +144,7 @@ static struct aa_profile *__attach_match(const char *name, int len = 0; struct aa_profile *profile, *candidate = NULL; - list_for_each_entry(profile, head, base.list) { + list_for_each_entry_rcu(profile, head, base.list) { if (profile->flags & PFLAG_NULL) continue; if (profile->xmatch && profile->xmatch_len > len) { @@ -177,9 +177,9 @@ static struct aa_profile *find_attach(struct aa_namespace *ns, { struct aa_profile *profile; - read_lock(&ns->lock); + rcu_read_lock(); profile = aa_get_profile(__attach_match(name, list)); - read_unlock(&ns->lock); + rcu_read_unlock(); return profile; } @@ -641,7 +641,10 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest) if (count) { /* attempting to change into a new hat or switch to a sibling */ struct aa_profile *root; - root = PROFILE_IS_HAT(profile) ? profile->parent : profile; + if (PROFILE_IS_HAT(profile)) + root = aa_get_profile_rcu(&profile->parent); + else + root = aa_get_profile(profile); /* find first matching hat */ for (i = 0; i < count && !hat; i++) @@ -653,6 +656,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest) error = -ECHILD; else error = -ENOENT; + aa_put_profile(root); goto out; } @@ -667,6 +671,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest) /* freed below */ name = new_compound_name(root->base.hname, hats[0]); + aa_put_profile(root); target = name; /* released below */ hat = aa_new_null_profile(profile, 1); @@ -676,6 +681,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest) goto audit; } } else { + aa_put_profile(root); target = hat->base.hname; if (!PROFILE_IS_HAT(hat)) { info = "target not hat"; diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h index 1ba2ca56a6ef..8fb1488a3cd4 100644 --- a/security/apparmor/include/apparmor.h +++ b/security/apparmor/include/apparmor.h @@ -78,6 +78,12 @@ static inline void *kvzalloc(size_t size) return __aa_kvmalloc(size, __GFP_ZERO); } +/* returns 0 if kref not incremented */ +static inline int kref_get_not0(struct kref *kref) +{ + return atomic_inc_not_zero(&kref->refcount); +} + /** * aa_strneq - compare null terminated @str to a non null terminated substring * @str: a null terminated string diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index b25491a3046a..82487a853353 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -42,6 +42,8 @@ extern const char *const profile_mode_names[]; #define PROFILE_IS_HAT(_profile) ((_profile)->flags & PFLAG_HAT) +#define on_list_rcu(X) (!list_empty(X) && (X)->prev != LIST_POISON2) + /* * FIXME: currently need a clean way to replace and remove profiles as a * set. It should be done at the namespace level. @@ -75,6 +77,7 @@ struct aa_profile; * @hname - The hierarchical name * @count: reference count of the obj * @list: list policy object is on + * @rcu: rcu head used when removing from @list * @profiles: head of the profiles list contained in the object */ struct aa_policy { @@ -83,6 +86,7 @@ struct aa_policy { struct kref count; struct list_head list; struct list_head profiles; + struct rcu_head rcu; }; /* struct aa_ns_acct - accounting of profiles in namespace @@ -124,7 +128,7 @@ struct aa_ns_acct { struct aa_namespace { struct aa_policy base; struct aa_namespace *parent; - rwlock_t lock; + struct mutex lock; struct aa_ns_acct acct; struct aa_profile *unconfined; struct list_head sub_ns; @@ -166,7 +170,7 @@ struct aa_policydb { * attachments are determined by profile X transition rules. * * The @replacedby field is write protected by the profile lock. Reads - * are assumed to be atomic, and are done without locking. + * are assumed to be atomic. * * Profiles have a hierarchy where hats and children profiles keep * a reference to their parent. @@ -177,7 +181,7 @@ struct aa_policydb { */ struct aa_profile { struct aa_policy base; - struct aa_profile *parent; + struct aa_profile __rcu *parent; struct aa_namespace *ns; struct aa_profile *replacedby; @@ -295,6 +299,41 @@ static inline struct aa_profile *aa_get_profile(struct aa_profile *p) return p; } +/** + * aa_get_profile_not0 - increment refcount on profile @p found via lookup + * @p: profile (MAYBE NULL) + * + * Returns: pointer to @p if @p is NULL will return NULL + * Requires: @p must be held with valid refcount when called + */ +static inline struct aa_profile *aa_get_profile_not0(struct aa_profile *p) +{ + if (p && kref_get_not0(&p->base.count)) + return p; + + return NULL; +} + +/** + * aa_get_profile_rcu - increment a refcount profile that can be replaced + * @p: pointer to profile that can be replaced (NOT NULL) + * + * Returns: pointer to a refcounted profile. + * else NULL if no profile + */ +static inline struct aa_profile *aa_get_profile_rcu(struct aa_profile __rcu **p) +{ + struct aa_profile *c; + + rcu_read_lock(); + do { + c = rcu_dereference(*p); + } while (c && !kref_get_not0(&c->base.count)); + rcu_read_unlock(); + + return c; +} + /** * aa_put_profile - decrement refcount on profile @p * @p: profile (MAYBE NULL) diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 407b442c0a2c..25bbbb482bb6 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -153,13 +153,13 @@ static bool policy_init(struct aa_policy *policy, const char *prefix, static void policy_destroy(struct aa_policy *policy) { /* still contains profiles -- invalid */ - if (!list_empty(&policy->profiles)) { + if (on_list_rcu(&policy->profiles)) { AA_ERROR("%s: internal error, " "policy '%s' still contains profiles\n", __func__, policy->name); BUG(); } - if (!list_empty(&policy->list)) { + if (on_list_rcu(&policy->list)) { AA_ERROR("%s: internal error, policy '%s' still on list\n", __func__, policy->name); BUG(); @@ -174,7 +174,7 @@ static void policy_destroy(struct aa_policy *policy) * @head: list to search (NOT NULL) * @name: name to search for (NOT NULL) * - * Requires: correct locks for the @head list be held + * Requires: rcu_read_lock be held * * Returns: unrefcounted policy that match @name or NULL if not found */ @@ -182,7 +182,7 @@ static struct aa_policy *__policy_find(struct list_head *head, const char *name) { struct aa_policy *policy; - list_for_each_entry(policy, head, list) { + list_for_each_entry_rcu(policy, head, list) { if (!strcmp(policy->name, name)) return policy; } @@ -195,7 +195,7 @@ static struct aa_policy *__policy_find(struct list_head *head, const char *name) * @str: string to search for (NOT NULL) * @len: length of match required * - * Requires: correct locks for the @head list be held + * Requires: rcu_read_lock be held * * Returns: unrefcounted policy that match @str or NULL if not found * @@ -207,7 +207,7 @@ static struct aa_policy *__policy_strn_find(struct list_head *head, { struct aa_policy *policy; - list_for_each_entry(policy, head, list) { + list_for_each_entry_rcu(policy, head, list) { if (aa_strneq(policy->name, str, len)) return policy; } @@ -284,7 +284,7 @@ static struct aa_namespace *alloc_namespace(const char *prefix, goto fail_ns; INIT_LIST_HEAD(&ns->sub_ns); - rwlock_init(&ns->lock); + mutex_init(&ns->lock); /* released by free_namespace */ ns->unconfined = aa_alloc_profile("unconfined"); @@ -350,7 +350,7 @@ void aa_free_namespace_kref(struct kref *kref) * * Returns: unrefcounted namespace * - * Requires: ns lock be held + * Requires: rcu_read_lock be held */ static struct aa_namespace *__aa_find_namespace(struct list_head *head, const char *name) @@ -373,9 +373,9 @@ struct aa_namespace *aa_find_namespace(struct aa_namespace *root, { struct aa_namespace *ns = NULL; - read_lock(&root->lock); + rcu_read_lock(); ns = aa_get_namespace(__aa_find_namespace(&root->sub_ns, name)); - read_unlock(&root->lock); + rcu_read_unlock(); return ns; } @@ -392,7 +392,7 @@ static struct aa_namespace *aa_prepare_namespace(const char *name) root = aa_current_profile()->ns; - write_lock(&root->lock); + mutex_lock(&root->lock); /* if name isn't specified the profile is loaded to the current ns */ if (!name) { @@ -405,31 +405,17 @@ static struct aa_namespace *aa_prepare_namespace(const char *name) /* released by caller */ ns = aa_get_namespace(__aa_find_namespace(&root->sub_ns, name)); if (!ns) { - /* namespace not found */ - struct aa_namespace *new_ns; - write_unlock(&root->lock); - new_ns = alloc_namespace(root->base.hname, name); - if (!new_ns) - return NULL; - write_lock(&root->lock); - /* test for race when new_ns was allocated */ - ns = __aa_find_namespace(&root->sub_ns, name); - if (!ns) { - /* add parent ref */ - new_ns->parent = aa_get_namespace(root); - - list_add(&new_ns->base.list, &root->sub_ns); - /* add list ref */ - ns = aa_get_namespace(new_ns); - } else { - /* raced so free the new one */ - free_namespace(new_ns); - /* get reference on namespace */ - aa_get_namespace(ns); - } + ns = alloc_namespace(root->base.hname, name); + if (!ns) + goto out; + /* add parent ref */ + ns->parent = aa_get_namespace(root); + list_add_rcu(&ns->base.list, &root->sub_ns); + /* add list ref */ + aa_get_namespace(ns); } out: - write_unlock(&root->lock); + mutex_unlock(&root->lock); /* return ref */ return ns; @@ -447,7 +433,7 @@ out: static void __list_add_profile(struct list_head *list, struct aa_profile *profile) { - list_add(&profile->base.list, list); + list_add_rcu(&profile->base.list, list); /* get list reference */ aa_get_profile(profile); } @@ -466,10 +452,8 @@ static void __list_add_profile(struct list_head *list, */ static void __list_remove_profile(struct aa_profile *profile) { - list_del_init(&profile->base.list); - if (!(profile->flags & PFLAG_NO_LIST_REF)) - /* release list reference */ - aa_put_profile(profile); + list_del_rcu(&profile->base.list); + aa_put_profile(profile); } static void __profile_list_release(struct list_head *head); @@ -510,17 +494,40 @@ static void __ns_list_release(struct list_head *head); */ static void destroy_namespace(struct aa_namespace *ns) { + struct aa_profile *unconfined; + if (!ns) return; - write_lock(&ns->lock); + mutex_lock(&ns->lock); /* release all profiles in this namespace */ __profile_list_release(&ns->base.profiles); /* release all sub namespaces */ __ns_list_release(&ns->sub_ns); - write_unlock(&ns->lock); + unconfined = ns->unconfined; + /* + * break the ns, unconfined profile cyclic reference and forward + * all new unconfined profiles requests to the parent namespace + * This will result in all confined tasks that have a profile + * being removed, inheriting the parent->unconfined profile. + */ + if (ns->parent) + ns->unconfined = aa_get_profile(ns->parent->unconfined); + + /* release original ns->unconfined ref */ + aa_put_profile(unconfined); + + mutex_unlock(&ns->lock); +} + +static void aa_put_ns_rcu(struct rcu_head *head) +{ + struct aa_namespace *ns = container_of(head, struct aa_namespace, + base.rcu); + /* release ns->base.list ref */ + aa_put_namespace(ns); } /** @@ -531,26 +538,12 @@ static void destroy_namespace(struct aa_namespace *ns) */ static void __remove_namespace(struct aa_namespace *ns) { - struct aa_profile *unconfined = ns->unconfined; - /* remove ns from namespace list */ - list_del_init(&ns->base.list); - - /* - * break the ns, unconfined profile cyclic reference and forward - * all new unconfined profiles requests to the parent namespace - * This will result in all confined tasks that have a profile - * being removed, inheriting the parent->unconfined profile. - */ - if (ns->parent) - ns->unconfined = aa_get_profile(ns->parent->unconfined); + list_del_rcu(&ns->base.list); destroy_namespace(ns); - /* release original ns->unconfined ref */ - aa_put_profile(unconfined); - /* release ns->base.list ref, from removal above */ - aa_put_namespace(ns); + call_rcu(&ns->base.rcu, aa_put_ns_rcu); } /** @@ -614,16 +607,9 @@ static void free_profile(struct aa_profile *profile) if (!profile) return; - if (!list_empty(&profile->base.list)) { - AA_ERROR("%s: internal error, " - "profile '%s' still on ns list\n", - __func__, profile->base.name); - BUG(); - } - /* free children profiles */ policy_destroy(&profile->base); - aa_put_profile(profile->parent); + aa_put_profile(rcu_access_pointer(profile->parent)); aa_put_namespace(profile->ns); kzfree(profile->rename); @@ -660,6 +646,16 @@ static void free_profile(struct aa_profile *profile) kzfree(profile); } +/** + * aa_free_profile_rcu - free aa_profile by rcu (called by aa_free_profile_kref) + * @head: rcu_head callback for freeing of a profile (NOT NULL) + */ +static void aa_free_profile_rcu(struct rcu_head *head) +{ + struct aa_profile *p = container_of(head, struct aa_profile, base.rcu); + free_profile(p); +} + /** * aa_free_profile_kref - free aa_profile by kref (called by aa_put_profile) * @kr: kref callback for freeing of a profile (NOT NULL) @@ -668,8 +664,7 @@ void aa_free_profile_kref(struct kref *kref) { struct aa_profile *p = container_of(kref, struct aa_profile, base.count); - - free_profile(p); + call_rcu(&p->base.rcu, aa_free_profile_rcu); } /** @@ -733,12 +728,12 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) profile->flags |= PFLAG_HAT; /* released on free_profile */ - profile->parent = aa_get_profile(parent); + rcu_assign_pointer(profile->parent, aa_get_profile(parent)); profile->ns = aa_get_namespace(parent->ns); - write_lock(&profile->ns->lock); + mutex_lock(&profile->ns->lock); __list_add_profile(&parent->base.profiles, profile); - write_unlock(&profile->ns->lock); + mutex_unlock(&profile->ns->lock); /* refcount released by caller */ return profile; @@ -754,7 +749,7 @@ fail: * @head: list to search (NOT NULL) * @name: name of profile (NOT NULL) * - * Requires: ns lock protecting list be held + * Requires: rcu_read_lock be held * * Returns: unrefcounted profile ptr, or NULL if not found */ @@ -769,7 +764,7 @@ static struct aa_profile *__find_child(struct list_head *head, const char *name) * @name: name of profile (NOT NULL) * @len: length of @name substring to match * - * Requires: ns lock protecting list be held + * Requires: rcu_read_lock be held * * Returns: unrefcounted profile ptr, or NULL if not found */ @@ -790,9 +785,9 @@ struct aa_profile *aa_find_child(struct aa_profile *parent, const char *name) { struct aa_profile *profile; - read_lock(&parent->ns->lock); + rcu_read_lock(); profile = aa_get_profile(__find_child(&parent->base.profiles, name)); - read_unlock(&parent->ns->lock); + rcu_read_unlock(); /* refcount released by caller */ return profile; @@ -807,7 +802,7 @@ struct aa_profile *aa_find_child(struct aa_profile *parent, const char *name) * that matches hname does not need to exist, in general this * is used to load a new profile. * - * Requires: ns->lock be held + * Requires: rcu_read_lock be held * * Returns: unrefcounted policy or NULL if not found */ @@ -839,7 +834,7 @@ static struct aa_policy *__lookup_parent(struct aa_namespace *ns, * @base: base list to start looking up profile name from (NOT NULL) * @hname: hierarchical profile name (NOT NULL) * - * Requires: ns->lock be held + * Requires: rcu_read_lock be held * * Returns: unrefcounted profile pointer or NULL if not found * @@ -878,9 +873,11 @@ struct aa_profile *aa_lookup_profile(struct aa_namespace *ns, const char *hname) { struct aa_profile *profile; - read_lock(&ns->lock); - profile = aa_get_profile(__lookup_profile(&ns->base, hname)); - read_unlock(&ns->lock); + rcu_read_lock(); + do { + profile = __lookup_profile(&ns->base, hname); + } while (profile && !aa_get_profile_not0(profile)); + rcu_read_unlock(); /* the unconfined profile is not in the regular profile list */ if (!profile && strcmp(hname, "unconfined") == 0) @@ -1002,7 +999,7 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new) if (!list_empty(&old->base.profiles)) { LIST_HEAD(lh); - list_splice_init(&old->base.profiles, &lh); + list_splice_init_rcu(&old->base.profiles, &lh, synchronize_rcu); list_for_each_entry_safe(child, tmp, &lh, base.list) { struct aa_profile *p; @@ -1018,20 +1015,24 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new) /* inherit @child and its children */ /* TODO: update hname of inherited children */ /* list refcount transferred to @new */ - list_add(&child->base.list, &new->base.profiles); - aa_put_profile(child->parent); - child->parent = aa_get_profile(new); + p = rcu_dereference_protected(child->parent, + mutex_is_locked(&child->ns->lock)); + rcu_assign_pointer(child->parent, aa_get_profile(new)); + list_add_rcu(&child->base.list, &new->base.profiles); + aa_put_profile(p); } } - if (!new->parent) - new->parent = aa_get_profile(old->parent); + if (!rcu_access_pointer(new->parent)) { + struct aa_profile *parent = rcu_dereference(old->parent); + rcu_assign_pointer(new->parent, aa_get_profile(parent)); + } /* released by free_profile */ old->replacedby = aa_get_profile(new); if (list_empty(&new->base.list)) { /* new is not on a list already */ - list_replace_init(&old->base.list, &new->base.list); + list_replace_rcu(&old->base.list, &new->base.list); aa_get_profile(new); aa_put_profile(old); } else @@ -1099,7 +1100,7 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) goto fail; } - write_lock(&ns->lock); + mutex_lock(&ns->lock); /* setup parent and ns info */ list_for_each_entry(ent, &lh, list) { struct aa_policy *policy; @@ -1135,11 +1136,12 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) name = ent->new->base.hname; goto fail_lock; } - ent->new->parent = aa_get_profile(p); - } else if (policy != &ns->base) + rcu_assign_pointer(ent->new->parent, aa_get_profile(p)); + } else if (policy != &ns->base) { /* released on profile replacement or free_profile */ - ent->new->parent = aa_get_profile((struct aa_profile *) - policy); + struct aa_profile *p = (struct aa_profile *) policy; + rcu_assign_pointer(ent->new->parent, aa_get_profile(p)); + } } /* do actual replacement */ @@ -1156,13 +1158,16 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) } else if (ent->rename) { __replace_profile(ent->rename, ent->new); } else if (ent->new->parent) { - struct aa_profile *parent; - parent = aa_newest_version(ent->new->parent); + struct aa_profile *parent, *newest; + parent = rcu_dereference_protected(ent->new->parent, + mutex_is_locked(&ns->lock)); + newest = aa_newest_version(parent); + /* parent replaced in this atomic set? */ - if (parent != ent->new->parent) { - aa_get_profile(parent); - aa_put_profile(ent->new->parent); - ent->new->parent = parent; + if (newest != parent) { + aa_get_profile(newest); + aa_put_profile(parent); + rcu_assign_pointer(ent->new->parent, newest); } __list_add_profile(&parent->base.profiles, ent->new); } else @@ -1170,7 +1175,7 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) aa_load_ent_free(ent); } - write_unlock(&ns->lock); + mutex_unlock(&ns->lock); out: aa_put_namespace(ns); @@ -1180,7 +1185,7 @@ out: return size; fail_lock: - write_unlock(&ns->lock); + mutex_unlock(&ns->lock); fail: error = audit_policy(op, GFP_KERNEL, name, info, error); @@ -1235,12 +1240,12 @@ ssize_t aa_remove_profiles(char *fqname, size_t size) if (!name) { /* remove namespace - can only happen if fqname[0] == ':' */ - write_lock(&ns->parent->lock); + mutex_lock(&ns->parent->lock); __remove_namespace(ns); - write_unlock(&ns->parent->lock); + mutex_unlock(&ns->parent->lock); } else { /* remove profile */ - write_lock(&ns->lock); + mutex_lock(&ns->lock); profile = aa_get_profile(__lookup_profile(&ns->base, name)); if (!profile) { error = -ENOENT; @@ -1249,7 +1254,7 @@ ssize_t aa_remove_profiles(char *fqname, size_t size) } name = profile->base.hname; __remove_profile(profile); - write_unlock(&ns->lock); + mutex_unlock(&ns->lock); } /* don't fail removal if audit fails */ @@ -1259,7 +1264,7 @@ ssize_t aa_remove_profiles(char *fqname, size_t size) return size; fail_ns_lock: - write_unlock(&ns->lock); + mutex_unlock(&ns->lock); aa_put_namespace(ns); fail: -- cgit v1.2.3 From 77b071b34045a0c65d0e1f85f3d47fd2b8b7a8a1 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:07:43 -0700 Subject: apparmor: change how profile replacement update is done 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 --- security/apparmor/context.c | 16 +++---- security/apparmor/domain.c | 4 +- security/apparmor/include/context.h | 15 +++---- security/apparmor/include/policy.h | 78 ++++++++++++++++++++++++---------- security/apparmor/lsm.c | 14 +++--- security/apparmor/policy.c | 85 ++++++++++++++++++++----------------- 6 files changed, 125 insertions(+), 87 deletions(-) (limited to 'security') diff --git a/security/apparmor/context.c b/security/apparmor/context.c index d5af1d15f26d..3064c6ced87c 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/context.c @@ -112,9 +112,9 @@ int aa_replace_current_profile(struct aa_profile *profile) aa_clear_task_cxt_trans(cxt); /* be careful switching cxt->profile, when racing replacement it - * is possible that cxt->profile->replacedby is the reference keeping - * @profile valid, so make sure to get its reference before dropping - * the reference on cxt->profile */ + * is possible that cxt->profile->replacedby->profile is the reference + * keeping @profile valid, so make sure to get its reference before + * dropping the reference on cxt->profile */ aa_get_profile(profile); aa_put_profile(cxt->profile); cxt->profile = profile; @@ -175,7 +175,7 @@ int aa_set_current_hat(struct aa_profile *profile, u64 token) abort_creds(new); return -EACCES; } - cxt->profile = aa_get_profile(aa_newest_version(profile)); + cxt->profile = aa_get_newest_profile(profile); /* clear exec on switching context */ aa_put_profile(cxt->onexec); cxt->onexec = NULL; @@ -212,14 +212,8 @@ int aa_restore_previous_profile(u64 token) } aa_put_profile(cxt->profile); - cxt->profile = aa_newest_version(cxt->previous); + cxt->profile = aa_get_newest_profile(cxt->previous); BUG_ON(!cxt->profile); - if (unlikely(cxt->profile != cxt->previous)) { - aa_get_profile(cxt->profile); - aa_put_profile(cxt->previous); - } - /* ref has been transfered so avoid putting ref in clear_task_cxt */ - cxt->previous = NULL; /* clear exec && prev information when restoring to previous context */ aa_clear_task_cxt_trans(cxt); diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 454bcd7f3452..5488d095af6f 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -359,7 +359,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) cxt = cred_cxt(bprm->cred); BUG_ON(!cxt); - profile = aa_get_profile(aa_newest_version(cxt->profile)); + profile = aa_get_newest_profile(cxt->profile); /* * get the namespace from the replacement profile as replacement * can change the namespace @@ -417,7 +417,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) if (!(cp.allow & AA_MAY_ONEXEC)) goto audit; - new_profile = aa_get_profile(aa_newest_version(cxt->onexec)); + new_profile = aa_get_newest_profile(cxt->onexec); goto apply; } diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h index d44ba5802e3d..6bf65798e5d1 100644 --- a/security/apparmor/include/context.h +++ b/security/apparmor/include/context.h @@ -98,7 +98,7 @@ static inline struct aa_profile *aa_cred_profile(const struct cred *cred) { struct aa_task_cxt *cxt = cred_cxt(cred); BUG_ON(!cxt || !cxt->profile); - return aa_newest_version(cxt->profile); + return cxt->profile; } /** @@ -152,15 +152,14 @@ static inline struct aa_profile *aa_current_profile(void) struct aa_profile *profile; BUG_ON(!cxt || !cxt->profile); - profile = aa_newest_version(cxt->profile); - /* - * Whether or not replacement succeeds, use newest profile so - * there is no need to update it after replacement. - */ - if (unlikely((cxt->profile != profile))) + if (PROFILE_INVALID(cxt->profile)) { + profile = aa_get_newest_profile(cxt->profile); aa_replace_current_profile(profile); + aa_put_profile(profile); + cxt = current_cxt(); + } - return profile; + return cxt->profile; } /** diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 82487a853353..e9f2baf4467e 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -42,6 +42,8 @@ extern const char *const profile_mode_names[]; #define PROFILE_IS_HAT(_profile) ((_profile)->flags & PFLAG_HAT) +#define PROFILE_INVALID(_profile) ((_profile)->flags & PFLAG_INVALID) + #define on_list_rcu(X) (!list_empty(X) && (X)->prev != LIST_POISON2) /* @@ -65,6 +67,7 @@ enum profile_flags { PFLAG_USER_DEFINED = 0x20, /* user based profile - lower privs */ PFLAG_NO_LIST_REF = 0x40, /* list doesn't keep profile ref */ PFLAG_OLD_NULL_TRANS = 0x100, /* use // as the null transition */ + PFLAG_INVALID = 0x200, /* profile replaced/removed */ /* These flags must correspond with PATH_flags */ PFLAG_MEDIATE_DELETED = 0x10000, /* mediate instead delegate deleted */ @@ -146,6 +149,12 @@ struct aa_policydb { }; +struct aa_replacedby { + struct kref count; + struct aa_profile __rcu *profile; +}; + + /* struct aa_profile - basic confinement data * @base - base components of the profile (name, refcount, lists, lock ...) * @parent: parent of profile @@ -169,8 +178,7 @@ struct aa_policydb { * used to determine profile attachment against unconfined tasks. All other * attachments are determined by profile X transition rules. * - * The @replacedby field is write protected by the profile lock. Reads - * are assumed to be atomic. + * The @replacedby struct is write protected by the profile lock. * * Profiles have a hierarchy where hats and children profiles keep * a reference to their parent. @@ -184,14 +192,14 @@ struct aa_profile { struct aa_profile __rcu *parent; struct aa_namespace *ns; - struct aa_profile *replacedby; + struct aa_replacedby *replacedby; const char *rename; struct aa_dfa *xmatch; int xmatch_len; enum audit_mode audit; enum profile_mode mode; - u32 flags; + long flags; u32 path_flags; int size; @@ -250,6 +258,7 @@ static inline void aa_put_namespace(struct aa_namespace *ns) kref_put(&ns->base.count, aa_free_namespace_kref); } +void aa_free_replacedby_kref(struct kref *kref); struct aa_profile *aa_alloc_profile(const char *name); struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat); void aa_free_profile_kref(struct kref *kref); @@ -265,24 +274,6 @@ ssize_t aa_remove_profiles(char *name, size_t size); #define unconfined(X) ((X)->flags & PFLAG_UNCONFINED) -/** - * aa_newest_version - find the newest version of @profile - * @profile: the profile to check for newer versions of (NOT NULL) - * - * Returns: newest version of @profile, if @profile is the newest version - * return @profile. - * - * NOTE: the profile returned is not refcounted, The refcount on @profile - * must be held until the caller decides what to do with the returned newest - * version. - */ -static inline struct aa_profile *aa_newest_version(struct aa_profile *profile) -{ - while (profile->replacedby) - profile = profile->replacedby; - - return profile; -} /** * aa_get_profile - increment refcount on profile @p @@ -334,6 +325,25 @@ static inline struct aa_profile *aa_get_profile_rcu(struct aa_profile __rcu **p) return c; } +/** + * aa_get_newest_profile - find the newest version of @profile + * @profile: the profile to check for newer versions of + * + * Returns: refcounted newest version of @profile taking into account + * replacement, renames and removals + * return @profile. + */ +static inline struct aa_profile *aa_get_newest_profile(struct aa_profile *p) +{ + if (!p) + return NULL; + + if (PROFILE_INVALID(p)) + return aa_get_profile_rcu(&p->replacedby->profile); + + return aa_get_profile(p); +} + /** * aa_put_profile - decrement refcount on profile @p * @p: profile (MAYBE NULL) @@ -344,6 +354,30 @@ static inline void aa_put_profile(struct aa_profile *p) kref_put(&p->base.count, aa_free_profile_kref); } +static inline struct aa_replacedby *aa_get_replacedby(struct aa_replacedby *p) +{ + if (p) + kref_get(&(p->count)); + + return p; +} + +static inline void aa_put_replacedby(struct aa_replacedby *p) +{ + if (p) + kref_put(&p->count, aa_free_replacedby_kref); +} + +/* requires profile list write lock held */ +static inline void __aa_update_replacedby(struct aa_profile *orig, + struct aa_profile *new) +{ + struct aa_profile *tmp = rcu_dereference(orig->replacedby->profile); + rcu_assign_pointer(orig->replacedby->profile, aa_get_profile(new)); + orig->flags |= PFLAG_INVALID; + aa_put_profile(tmp); +} + static inline int AUDIT_MODE(struct aa_profile *profile) { if (aa_g_audit != AUDIT_NORMAL) diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 96506dfe51ec..c8c148a738f7 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -508,19 +508,21 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, /* released below */ const struct cred *cred = get_task_cred(task); struct aa_task_cxt *cxt = cred_cxt(cred); + struct aa_profile *profile = NULL; if (strcmp(name, "current") == 0) - error = aa_getprocattr(aa_newest_version(cxt->profile), - value); + profile = aa_get_newest_profile(cxt->profile); else if (strcmp(name, "prev") == 0 && cxt->previous) - error = aa_getprocattr(aa_newest_version(cxt->previous), - value); + profile = aa_get_newest_profile(cxt->previous); else if (strcmp(name, "exec") == 0 && cxt->onexec) - error = aa_getprocattr(aa_newest_version(cxt->onexec), - value); + profile = aa_get_newest_profile(cxt->onexec); else error = -EINVAL; + if (profile) + error = aa_getprocattr(profile, value); + + aa_put_profile(profile); put_cred(cred); return error; diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 25bbbb482bb6..41b8f275c626 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -469,7 +469,7 @@ static void __remove_profile(struct aa_profile *profile) /* release any children lists first */ __profile_list_release(&profile->base.profiles); /* released by free_profile */ - profile->replacedby = aa_get_profile(profile->ns->unconfined); + __aa_update_replacedby(profile, profile->ns->unconfined); __list_remove_profile(profile); } @@ -588,6 +588,23 @@ void __init aa_free_root_ns(void) aa_put_namespace(ns); } + +static void free_replacedby(struct aa_replacedby *r) +{ + if (r) { + aa_put_profile(rcu_dereference(r->profile)); + kzfree(r); + } +} + + +void aa_free_replacedby_kref(struct kref *kref) +{ + struct aa_replacedby *r = container_of(kref, struct aa_replacedby, + count); + free_replacedby(r); +} + /** * free_profile - free a profile * @profile: the profile to free (MAYBE NULL) @@ -600,8 +617,6 @@ void __init aa_free_root_ns(void) */ static void free_profile(struct aa_profile *profile) { - struct aa_profile *p; - AA_DEBUG("%s(%p)\n", __func__, profile); if (!profile) @@ -620,28 +635,7 @@ static void free_profile(struct aa_profile *profile) aa_put_dfa(profile->xmatch); aa_put_dfa(profile->policy.dfa); - - /* put the profile reference for replacedby, but not via - * put_profile(kref_put). - * replacedby can form a long chain that can result in cascading - * frees that blows the stack because kref_put makes a nested fn - * call (it looks like recursion, with free_profile calling - * free_profile) for each profile in the chain lp#1056078. - */ - for (p = profile->replacedby; p; ) { - if (atomic_dec_and_test(&p->base.count.refcount)) { - /* no more refs on p, grab its replacedby */ - struct aa_profile *next = p->replacedby; - /* break the chain */ - p->replacedby = NULL; - /* now free p, chain is broken */ - free_profile(p); - - /* follow up with next profile in the chain */ - p = next; - } else - break; - } + aa_put_replacedby(profile->replacedby); kzfree(profile); } @@ -682,13 +676,22 @@ struct aa_profile *aa_alloc_profile(const char *hname) if (!profile) return NULL; - if (!policy_init(&profile->base, NULL, hname)) { - kzfree(profile); - return NULL; - } + profile->replacedby = kzalloc(sizeof(struct aa_replacedby), GFP_KERNEL); + if (!profile->replacedby) + goto fail; + kref_init(&profile->replacedby->count); + + if (!policy_init(&profile->base, NULL, hname)) + goto fail; /* refcount released by caller */ return profile; + +fail: + kzfree(profile->replacedby); + kzfree(profile); + + return NULL; } /** @@ -985,6 +988,7 @@ static struct aa_profile *__list_lookup_parent(struct list_head *lh, * __replace_profile - replace @old with @new on a list * @old: profile to be replaced (NOT NULL) * @new: profile to replace @old with (NOT NULL) + * @share_replacedby: transfer @old->replacedby to @new * * Will duplicate and refcount elements that @new inherits from @old * and will inherit @old children. @@ -993,7 +997,8 @@ static struct aa_profile *__list_lookup_parent(struct list_head *lh, * * Requires: namespace list lock be held, or list not be shared */ -static void __replace_profile(struct aa_profile *old, struct aa_profile *new) +static void __replace_profile(struct aa_profile *old, struct aa_profile *new, + bool share_replacedby) { struct aa_profile *child, *tmp; @@ -1008,7 +1013,7 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new) p = __find_child(&new->base.profiles, child->base.name); if (p) { /* @p replaces @child */ - __replace_profile(child, p); + __replace_profile(child, p, share_replacedby); continue; } @@ -1027,8 +1032,11 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new) struct aa_profile *parent = rcu_dereference(old->parent); rcu_assign_pointer(new->parent, aa_get_profile(parent)); } - /* released by free_profile */ - old->replacedby = aa_get_profile(new); + __aa_update_replacedby(old, new); + if (share_replacedby) { + aa_put_replacedby(new->replacedby); + new->replacedby = aa_get_replacedby(old->replacedby); + } if (list_empty(&new->base.list)) { /* new is not on a list already */ @@ -1152,23 +1160,24 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) audit_policy(op, GFP_ATOMIC, ent->new->base.name, NULL, error); if (ent->old) { - __replace_profile(ent->old, ent->new); + __replace_profile(ent->old, ent->new, 1); if (ent->rename) - __replace_profile(ent->rename, ent->new); + __replace_profile(ent->rename, ent->new, 0); } else if (ent->rename) { - __replace_profile(ent->rename, ent->new); + __replace_profile(ent->rename, ent->new, 0); } else if (ent->new->parent) { struct aa_profile *parent, *newest; parent = rcu_dereference_protected(ent->new->parent, mutex_is_locked(&ns->lock)); - newest = aa_newest_version(parent); + newest = aa_get_newest_profile(parent); /* parent replaced in this atomic set? */ if (newest != parent) { aa_get_profile(newest); aa_put_profile(parent); rcu_assign_pointer(ent->new->parent, newest); - } + } else + aa_put_profile(newest); __list_add_profile(&parent->base.profiles, ent->new); } else __list_add_profile(&ns->base.profiles, ent->new); -- cgit v1.2.3 From fa2ac468db510c653499a47c1ec3deb045bf4763 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:08:43 -0700 Subject: apparmor: update how unconfined is handled 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 Acked-by: Seth Arnold --- security/apparmor/domain.c | 2 +- security/apparmor/include/policy.h | 80 +++++++++++++++++++------------------- security/apparmor/policy.c | 68 +++++++++++++------------------- 3 files changed, 67 insertions(+), 83 deletions(-) (limited to 'security') diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 5488d095af6f..bc28f2670ee4 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -434,7 +434,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) new_profile = aa_get_profile(profile); goto x_clear; } else if (perms.xindex & AA_X_UNCONFINED) { - new_profile = aa_get_profile(ns->unconfined); + new_profile = aa_get_newest_profile(ns->unconfined); info = "ux fallback"; } else { error = -ENOENT; diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index e9f2baf4467e..1ddd5e5728b8 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -68,6 +68,7 @@ enum profile_flags { PFLAG_NO_LIST_REF = 0x40, /* list doesn't keep profile ref */ PFLAG_OLD_NULL_TRANS = 0x100, /* use // as the null transition */ PFLAG_INVALID = 0x200, /* profile replaced/removed */ + PFLAG_NS_COUNT = 0x400, /* carries NS ref count */ /* These flags must correspond with PATH_flags */ PFLAG_MEDIATE_DELETED = 0x10000, /* mediate instead delegate deleted */ @@ -78,7 +79,6 @@ struct aa_profile; /* struct aa_policy - common part of both namespaces and profiles * @name: name of the object * @hname - The hierarchical name - * @count: reference count of the obj * @list: list policy object is on * @rcu: rcu head used when removing from @list * @profiles: head of the profiles list contained in the object @@ -86,7 +86,6 @@ struct aa_profile; struct aa_policy { char *name; char *hname; - struct kref count; struct list_head list; struct list_head profiles; struct rcu_head rcu; @@ -157,6 +156,7 @@ struct aa_replacedby { /* struct aa_profile - basic confinement data * @base - base components of the profile (name, refcount, lists, lock ...) + * @count: reference count of the obj * @parent: parent of profile * @ns: namespace the profile is in * @replacedby: is set to the profile that replaced this profile @@ -189,6 +189,7 @@ struct aa_replacedby { */ struct aa_profile { struct aa_policy base; + struct kref count; struct aa_profile __rcu *parent; struct aa_namespace *ns; @@ -223,40 +224,6 @@ void aa_free_namespace_kref(struct kref *kref); struct aa_namespace *aa_find_namespace(struct aa_namespace *root, const char *name); -static inline struct aa_policy *aa_get_common(struct aa_policy *c) -{ - if (c) - kref_get(&c->count); - - return c; -} - -/** - * aa_get_namespace - increment references count on @ns - * @ns: namespace to increment reference count of (MAYBE NULL) - * - * Returns: pointer to @ns, if @ns is NULL returns NULL - * Requires: @ns must be held with valid refcount when called - */ -static inline struct aa_namespace *aa_get_namespace(struct aa_namespace *ns) -{ - if (ns) - kref_get(&(ns->base.count)); - - return ns; -} - -/** - * aa_put_namespace - decrement refcount on @ns - * @ns: namespace to put reference of - * - * Decrement reference count of @ns and if no longer in use free it - */ -static inline void aa_put_namespace(struct aa_namespace *ns) -{ - if (ns) - kref_put(&ns->base.count, aa_free_namespace_kref); -} void aa_free_replacedby_kref(struct kref *kref); struct aa_profile *aa_alloc_profile(const char *name); @@ -285,7 +252,7 @@ ssize_t aa_remove_profiles(char *name, size_t size); static inline struct aa_profile *aa_get_profile(struct aa_profile *p) { if (p) - kref_get(&(p->base.count)); + kref_get(&(p->count)); return p; } @@ -299,7 +266,7 @@ static inline struct aa_profile *aa_get_profile(struct aa_profile *p) */ static inline struct aa_profile *aa_get_profile_not0(struct aa_profile *p) { - if (p && kref_get_not0(&p->base.count)) + if (p && kref_get_not0(&p->count)) return p; return NULL; @@ -319,7 +286,7 @@ static inline struct aa_profile *aa_get_profile_rcu(struct aa_profile __rcu **p) rcu_read_lock(); do { c = rcu_dereference(*p); - } while (c && !kref_get_not0(&c->base.count)); + } while (c && !kref_get_not0(&c->count)); rcu_read_unlock(); return c; @@ -350,8 +317,12 @@ static inline struct aa_profile *aa_get_newest_profile(struct aa_profile *p) */ static inline void aa_put_profile(struct aa_profile *p) { - if (p) - kref_put(&p->base.count, aa_free_profile_kref); + if (p) { + if (p->flags & PFLAG_NS_COUNT) + kref_put(&p->count, aa_free_namespace_kref); + else + kref_put(&p->count, aa_free_profile_kref); + } } static inline struct aa_replacedby *aa_get_replacedby(struct aa_replacedby *p) @@ -378,6 +349,33 @@ static inline void __aa_update_replacedby(struct aa_profile *orig, aa_put_profile(tmp); } +/** + * aa_get_namespace - increment references count on @ns + * @ns: namespace to increment reference count of (MAYBE NULL) + * + * Returns: pointer to @ns, if @ns is NULL returns NULL + * Requires: @ns must be held with valid refcount when called + */ +static inline struct aa_namespace *aa_get_namespace(struct aa_namespace *ns) +{ + if (ns) + aa_get_profile(ns->unconfined); + + return ns; +} + +/** + * aa_put_namespace - decrement refcount on @ns + * @ns: namespace to put reference of + * + * Decrement reference count of @ns and if no longer in use free it + */ +static inline void aa_put_namespace(struct aa_namespace *ns) +{ + if (ns) + aa_put_profile(ns->unconfined); +} + static inline int AUDIT_MODE(struct aa_profile *profile) { if (aa_g_audit != AUDIT_NORMAL) diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 41b8f275c626..0ceee967434c 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -141,7 +141,6 @@ static bool policy_init(struct aa_policy *policy, const char *prefix, policy->name = (char *)hname_tail(policy->hname); INIT_LIST_HEAD(&policy->list); INIT_LIST_HEAD(&policy->profiles); - kref_init(&policy->count); return 1; } @@ -292,14 +291,10 @@ static struct aa_namespace *alloc_namespace(const char *prefix, goto fail_unconfined; ns->unconfined->flags = PFLAG_UNCONFINED | PFLAG_IX_ON_NAME_ERROR | - PFLAG_IMMUTABLE; + PFLAG_IMMUTABLE | PFLAG_NS_COUNT; - /* - * released by free_namespace, however __remove_namespace breaks - * the cyclic references (ns->unconfined, and unconfined->ns) and - * replaces with refs to parent namespace unconfined - */ - ns->unconfined->ns = aa_get_namespace(ns); + /* ns and ns->unconfined share ns->unconfined refcount */ + ns->unconfined->ns = ns; atomic_set(&ns->uniq_null, 0); @@ -312,6 +307,7 @@ fail_ns: return NULL; } +static void free_profile(struct aa_profile *profile); /** * free_namespace - free a profile namespace * @ns: the namespace to free (MAYBE NULL) @@ -327,20 +323,33 @@ static void free_namespace(struct aa_namespace *ns) policy_destroy(&ns->base); aa_put_namespace(ns->parent); - if (ns->unconfined && ns->unconfined->ns == ns) - ns->unconfined->ns = NULL; - - aa_put_profile(ns->unconfined); + ns->unconfined->ns = NULL; + free_profile(ns->unconfined); kzfree(ns); } +/** + * aa_free_namespace_rcu - free aa_namespace by rcu + * @head: rcu_head callback for freeing of a profile (NOT NULL) + * + * rcu_head is to the unconfined profile associated with the namespace + */ +static void aa_free_namespace_rcu(struct rcu_head *head) +{ + struct aa_profile *p = container_of(head, struct aa_profile, base.rcu); + free_namespace(p->ns); +} + /** * aa_free_namespace_kref - free aa_namespace by kref (see aa_put_namespace) * @kr: kref callback for freeing of a namespace (NOT NULL) + * + * kref is to the unconfined profile associated with the namespace */ void aa_free_namespace_kref(struct kref *kref) { - free_namespace(container_of(kref, struct aa_namespace, base.count)); + struct aa_profile *p = container_of(kref, struct aa_profile, count); + call_rcu(&p->base.rcu, aa_free_namespace_rcu); } /** @@ -494,8 +503,6 @@ static void __ns_list_release(struct list_head *head); */ static void destroy_namespace(struct aa_namespace *ns) { - struct aa_profile *unconfined; - if (!ns) return; @@ -506,30 +513,11 @@ static void destroy_namespace(struct aa_namespace *ns) /* release all sub namespaces */ __ns_list_release(&ns->sub_ns); - unconfined = ns->unconfined; - /* - * break the ns, unconfined profile cyclic reference and forward - * all new unconfined profiles requests to the parent namespace - * This will result in all confined tasks that have a profile - * being removed, inheriting the parent->unconfined profile. - */ if (ns->parent) - ns->unconfined = aa_get_profile(ns->parent->unconfined); - - /* release original ns->unconfined ref */ - aa_put_profile(unconfined); - + __aa_update_replacedby(ns->unconfined, ns->parent->unconfined); mutex_unlock(&ns->lock); } -static void aa_put_ns_rcu(struct rcu_head *head) -{ - struct aa_namespace *ns = container_of(head, struct aa_namespace, - base.rcu); - /* release ns->base.list ref */ - aa_put_namespace(ns); -} - /** * __remove_namespace - remove a namespace and all its children * @ns: namespace to be removed (NOT NULL) @@ -540,10 +528,8 @@ static void __remove_namespace(struct aa_namespace *ns) { /* remove ns from namespace list */ list_del_rcu(&ns->base.list); - destroy_namespace(ns); - - call_rcu(&ns->base.rcu, aa_put_ns_rcu); + aa_put_namespace(ns); } /** @@ -656,8 +642,7 @@ static void aa_free_profile_rcu(struct rcu_head *head) */ void aa_free_profile_kref(struct kref *kref) { - struct aa_profile *p = container_of(kref, struct aa_profile, - base.count); + struct aa_profile *p = container_of(kref, struct aa_profile, count); call_rcu(&p->base.rcu, aa_free_profile_rcu); } @@ -683,6 +668,7 @@ struct aa_profile *aa_alloc_profile(const char *hname) if (!policy_init(&profile->base, NULL, hname)) goto fail; + kref_init(&profile->count); /* refcount released by caller */ return profile; @@ -884,7 +870,7 @@ struct aa_profile *aa_lookup_profile(struct aa_namespace *ns, const char *hname) /* the unconfined profile is not in the regular profile list */ if (!profile && strcmp(hname, "unconfined") == 0) - profile = aa_get_profile(ns->unconfined); + profile = aa_get_newest_profile(ns->unconfined); /* refcount released by caller */ return profile; -- cgit v1.2.3 From 742058b0f3a2ed32e2a7349aff97989dc4e32452 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:10:43 -0700 Subject: apparmor: rework namespace free path 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 Acked-by: Seth Arnold --- security/apparmor/include/policy.h | 12 ++++-------- security/apparmor/policy.c | 33 ++++++--------------------------- 2 files changed, 10 insertions(+), 35 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 1ddd5e5728b8..4eafdd88f44e 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -80,7 +80,6 @@ struct aa_profile; * @name: name of the object * @hname - The hierarchical name * @list: list policy object is on - * @rcu: rcu head used when removing from @list * @profiles: head of the profiles list contained in the object */ struct aa_policy { @@ -88,7 +87,6 @@ struct aa_policy { char *hname; struct list_head list; struct list_head profiles; - struct rcu_head rcu; }; /* struct aa_ns_acct - accounting of profiles in namespace @@ -157,6 +155,7 @@ struct aa_replacedby { /* struct aa_profile - basic confinement data * @base - base components of the profile (name, refcount, lists, lock ...) * @count: reference count of the obj + * @rcu: rcu head used when removing from @list * @parent: parent of profile * @ns: namespace the profile is in * @replacedby: is set to the profile that replaced this profile @@ -190,6 +189,7 @@ struct aa_replacedby { struct aa_profile { struct aa_policy base; struct kref count; + struct rcu_head rcu; struct aa_profile __rcu *parent; struct aa_namespace *ns; @@ -317,12 +317,8 @@ static inline struct aa_profile *aa_get_newest_profile(struct aa_profile *p) */ static inline void aa_put_profile(struct aa_profile *p) { - if (p) { - if (p->flags & PFLAG_NS_COUNT) - kref_put(&p->count, aa_free_namespace_kref); - else - kref_put(&p->count, aa_free_profile_kref); - } + if (p) + kref_put(&p->count, aa_free_profile_kref); } static inline struct aa_replacedby *aa_get_replacedby(struct aa_replacedby *p) diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 0ceee967434c..aee2e71827cd 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -328,30 +328,6 @@ static void free_namespace(struct aa_namespace *ns) kzfree(ns); } -/** - * aa_free_namespace_rcu - free aa_namespace by rcu - * @head: rcu_head callback for freeing of a profile (NOT NULL) - * - * rcu_head is to the unconfined profile associated with the namespace - */ -static void aa_free_namespace_rcu(struct rcu_head *head) -{ - struct aa_profile *p = container_of(head, struct aa_profile, base.rcu); - free_namespace(p->ns); -} - -/** - * aa_free_namespace_kref - free aa_namespace by kref (see aa_put_namespace) - * @kr: kref callback for freeing of a namespace (NOT NULL) - * - * kref is to the unconfined profile associated with the namespace - */ -void aa_free_namespace_kref(struct kref *kref) -{ - struct aa_profile *p = container_of(kref, struct aa_profile, count); - call_rcu(&p->base.rcu, aa_free_namespace_rcu); -} - /** * __aa_find_namespace - find a namespace on a list by @name * @head: list to search for namespace on (NOT NULL) @@ -632,8 +608,11 @@ static void free_profile(struct aa_profile *profile) */ static void aa_free_profile_rcu(struct rcu_head *head) { - struct aa_profile *p = container_of(head, struct aa_profile, base.rcu); - free_profile(p); + struct aa_profile *p = container_of(head, struct aa_profile, rcu); + if (p->flags & PFLAG_NS_COUNT) + free_namespace(p->ns); + else + free_profile(p); } /** @@ -643,7 +622,7 @@ static void aa_free_profile_rcu(struct rcu_head *head) void aa_free_profile_kref(struct kref *kref) { struct aa_profile *p = container_of(kref, struct aa_profile, count); - call_rcu(&p->base.rcu, aa_free_profile_rcu); + call_rcu(&p->rcu, aa_free_profile_rcu); } /** -- cgit v1.2.3 From 8651e1d6572bc2c061073f05fabcd7175789259d Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:11:43 -0700 Subject: apparmor: make free_profile available outside of policy.c Signed-off-by: John Johansen --- security/apparmor/include/policy.h | 1 + security/apparmor/policy.c | 9 ++++----- security/apparmor/policy_unpack.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 4eafdd88f44e..8a68226ff7f7 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -228,6 +228,7 @@ struct aa_namespace *aa_find_namespace(struct aa_namespace *root, void aa_free_replacedby_kref(struct kref *kref); struct aa_profile *aa_alloc_profile(const char *name); struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat); +void aa_free_profile(struct aa_profile *profile); void aa_free_profile_kref(struct kref *kref); struct aa_profile *aa_find_child(struct aa_profile *parent, const char *name); struct aa_profile *aa_lookup_profile(struct aa_namespace *ns, const char *name); diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index aee2e71827cd..7a80b0c7e0ce 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -307,7 +307,6 @@ fail_ns: return NULL; } -static void free_profile(struct aa_profile *profile); /** * free_namespace - free a profile namespace * @ns: the namespace to free (MAYBE NULL) @@ -324,7 +323,7 @@ static void free_namespace(struct aa_namespace *ns) aa_put_namespace(ns->parent); ns->unconfined->ns = NULL; - free_profile(ns->unconfined); + aa_free_profile(ns->unconfined); kzfree(ns); } @@ -568,7 +567,7 @@ void aa_free_replacedby_kref(struct kref *kref) } /** - * free_profile - free a profile + * aa_free_profile - free a profile * @profile: the profile to free (MAYBE NULL) * * Free a profile, its hats and null_profile. All references to the profile, @@ -577,7 +576,7 @@ void aa_free_replacedby_kref(struct kref *kref) * If the profile was referenced from a task context, free_profile() will * be called from an rcu callback routine, so we must not sleep here. */ -static void free_profile(struct aa_profile *profile) +void aa_free_profile(struct aa_profile *profile) { AA_DEBUG("%s(%p)\n", __func__, profile); @@ -612,7 +611,7 @@ static void aa_free_profile_rcu(struct rcu_head *head) if (p->flags & PFLAG_NS_COUNT) free_namespace(p->ns); else - free_profile(p); + aa_free_profile(p); } /** diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 080a26b11f01..ce15313896ee 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -616,7 +616,7 @@ fail: else if (!name) name = "unknown"; audit_iface(profile, name, "failed to unpack profile", e, error); - aa_put_profile(profile); + aa_free_profile(profile); return ERR_PTR(error); } @@ -763,7 +763,7 @@ int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns) error = verify_profile(profile); if (error) { - aa_put_profile(profile); + aa_free_profile(profile); goto fail; } -- cgit v1.2.3 From 038165070aa55375d4bdd2f84b34a486feca63d6 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:12:43 -0700 Subject: apparmor: allow setting any profile into the unconfined state 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 Acked-by: Seth Arnold --- security/apparmor/domain.c | 4 ++-- security/apparmor/include/policy.h | 6 +++--- security/apparmor/include/policy_unpack.h | 7 +++++++ security/apparmor/policy.c | 6 ++++-- security/apparmor/policy_unpack.c | 8 ++++++-- 5 files changed, 22 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index bc28f2670ee4..26c607c971f5 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -371,8 +371,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) error = aa_path_name(&bprm->file->f_path, profile->path_flags, &buffer, &name, &info); if (error) { - if (profile->flags & - (PFLAG_IX_ON_NAME_ERROR | PFLAG_UNCONFINED)) + if (unconfined(profile) || + (profile->flags & PFLAG_IX_ON_NAME_ERROR)) error = 0; name = bprm->filename; goto audit; diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 8a68226ff7f7..65662e3c75cf 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -56,11 +56,11 @@ enum profile_mode { APPARMOR_ENFORCE, /* enforce access rules */ APPARMOR_COMPLAIN, /* allow and log access violations */ APPARMOR_KILL, /* kill task on access violation */ + APPARMOR_UNCONFINED, /* profile set to unconfined */ }; enum profile_flags { PFLAG_HAT = 1, /* profile is a hat */ - PFLAG_UNCONFINED = 2, /* profile is an unconfined profile */ PFLAG_NULL = 4, /* profile is null learning profile */ PFLAG_IX_ON_NAME_ERROR = 8, /* fallback to ix on name lookup fail */ PFLAG_IMMUTABLE = 0x10, /* don't allow changes/replacement */ @@ -199,7 +199,7 @@ struct aa_profile { struct aa_dfa *xmatch; int xmatch_len; enum audit_mode audit; - enum profile_mode mode; + long mode; long flags; u32 path_flags; int size; @@ -240,7 +240,7 @@ ssize_t aa_remove_profiles(char *name, size_t size); #define PROF_ADD 1 #define PROF_REPLACE 0 -#define unconfined(X) ((X)->flags & PFLAG_UNCONFINED) +#define unconfined(X) ((X)->mode == APPARMOR_UNCONFINED) /** diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h index 0d7ad722b8ff..c214fb88b1bc 100644 --- a/security/apparmor/include/policy_unpack.h +++ b/security/apparmor/include/policy_unpack.h @@ -27,6 +27,13 @@ struct aa_load_ent { void aa_load_ent_free(struct aa_load_ent *ent); struct aa_load_ent *aa_load_ent_alloc(void); +#define PACKED_FLAG_HAT 1 + +#define PACKED_MODE_ENFORCE 0 +#define PACKED_MODE_COMPLAIN 1 +#define PACKED_MODE_KILL 2 +#define PACKED_MODE_UNCONFINED 3 + int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns); #endif /* __POLICY_INTERFACE_H */ diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 7a80b0c7e0ce..2e4e2ecb25bc 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -96,6 +96,7 @@ const char *const profile_mode_names[] = { "enforce", "complain", "kill", + "unconfined", }; /** @@ -290,8 +291,9 @@ static struct aa_namespace *alloc_namespace(const char *prefix, if (!ns->unconfined) goto fail_unconfined; - ns->unconfined->flags = PFLAG_UNCONFINED | PFLAG_IX_ON_NAME_ERROR | - PFLAG_IMMUTABLE | PFLAG_NS_COUNT; + ns->unconfined->flags = PFLAG_IX_ON_NAME_ERROR | + PFLAG_IMMUTABLE | PFLAG_NS_COUNT; + ns->unconfined->mode = APPARMOR_UNCONFINED; /* ns and ns->unconfined share ns->unconfined refcount */ ns->unconfined->ns = ns; diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index ce15313896ee..cac0aa075787 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -511,12 +511,16 @@ static struct aa_profile *unpack_profile(struct aa_ext *e) goto fail; if (!unpack_u32(e, &tmp, NULL)) goto fail; - if (tmp) + if (tmp & PACKED_FLAG_HAT) profile->flags |= PFLAG_HAT; if (!unpack_u32(e, &tmp, NULL)) goto fail; - if (tmp) + if (tmp == PACKED_MODE_COMPLAIN) profile->mode = APPARMOR_COMPLAIN; + else if (tmp == PACKED_MODE_KILL) + profile->mode = APPARMOR_KILL; + else if (tmp == PACKED_MODE_UNCONFINED) + profile->mode = APPARMOR_UNCONFINED; if (!unpack_u32(e, &tmp, NULL)) goto fail; if (tmp) -- cgit v1.2.3 From 0d259f043f5f60f74c4fd020aac190cb6450e918 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:13:43 -0700 Subject: apparmor: add interface files for profiles and namespaces 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 --- security/apparmor/apparmorfs.c | 322 ++++++++++++++++++++++++++++++++- security/apparmor/include/apparmorfs.h | 38 ++++ security/apparmor/include/audit.h | 1 - security/apparmor/include/policy.h | 21 ++- security/apparmor/lsm.c | 6 +- security/apparmor/policy.c | 75 ++++++-- security/apparmor/procattr.c | 2 +- 7 files changed, 436 insertions(+), 29 deletions(-) (limited to 'security') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 3ed56e21a9fd..0fdd08c6ea59 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -12,6 +12,7 @@ * License. */ +#include #include #include #include @@ -27,6 +28,45 @@ #include "include/policy.h" #include "include/resource.h" +/** + * aa_mangle_name - mangle a profile name to std profile layout form + * @name: profile name to mangle (NOT NULL) + * @target: buffer to store mangled name, same length as @name (MAYBE NULL) + * + * Returns: length of mangled name + */ +static int mangle_name(char *name, char *target) +{ + char *t = target; + + while (*name == '/' || *name == '.') + name++; + + if (target) { + for (; *name; name++) { + if (*name == '/') + *(t)++ = '.'; + else if (isspace(*name)) + *(t)++ = '_'; + else if (isalnum(*name) || strchr("._-", *name)) + *(t)++ = *name; + } + + *t = 0; + } else { + int len = 0; + for (; *name; name++) { + if (isalnum(*name) || isspace(*name) || + strchr("/._-", *name)) + len++; + } + + return len; + } + + return t - target; +} + /** * aa_simple_write_to_buffer - common routine for getting policy from user * @op: operation doing the user buffer copy @@ -182,8 +222,263 @@ const struct file_operations aa_fs_seq_file_ops = { .release = single_release, }; -/** Base file system setup **/ +static int aa_fs_seq_profile_open(struct inode *inode, struct file *file, + int (*show)(struct seq_file *, void *)) +{ + struct aa_replacedby *r = aa_get_replacedby(inode->i_private); + int error = single_open(file, show, r); + + if (error) { + file->private_data = NULL; + aa_put_replacedby(r); + } + + return error; +} + +static int aa_fs_seq_profile_release(struct inode *inode, struct file *file) +{ + struct seq_file *seq = (struct seq_file *) file->private_data; + if (seq) + aa_put_replacedby(seq->private); + return single_release(inode, file); +} + +static int aa_fs_seq_profname_show(struct seq_file *seq, void *v) +{ + struct aa_replacedby *r = seq->private; + struct aa_profile *profile = aa_get_profile_rcu(&r->profile); + seq_printf(seq, "%s\n", profile->base.name); + aa_put_profile(profile); + + return 0; +} + +static int aa_fs_seq_profname_open(struct inode *inode, struct file *file) +{ + return aa_fs_seq_profile_open(inode, file, aa_fs_seq_profname_show); +} + +static const struct file_operations aa_fs_profname_fops = { + .owner = THIS_MODULE, + .open = aa_fs_seq_profname_open, + .read = seq_read, + .llseek = seq_lseek, + .release = aa_fs_seq_profile_release, +}; + +static int aa_fs_seq_profmode_show(struct seq_file *seq, void *v) +{ + struct aa_replacedby *r = seq->private; + struct aa_profile *profile = aa_get_profile_rcu(&r->profile); + seq_printf(seq, "%s\n", aa_profile_mode_names[profile->mode]); + aa_put_profile(profile); + + return 0; +} + +static int aa_fs_seq_profmode_open(struct inode *inode, struct file *file) +{ + return aa_fs_seq_profile_open(inode, file, aa_fs_seq_profmode_show); +} + +static const struct file_operations aa_fs_profmode_fops = { + .owner = THIS_MODULE, + .open = aa_fs_seq_profmode_open, + .read = seq_read, + .llseek = seq_lseek, + .release = aa_fs_seq_profile_release, +}; + +/** fns to setup dynamic per profile/namespace files **/ +void __aa_fs_profile_rmdir(struct aa_profile *profile) +{ + struct aa_profile *child; + int i; + + if (!profile) + return; + + list_for_each_entry(child, &profile->base.profiles, base.list) + __aa_fs_profile_rmdir(child); + + for (i = AAFS_PROF_SIZEOF - 1; i >= 0; --i) { + struct aa_replacedby *r; + if (!profile->dents[i]) + continue; + + r = profile->dents[i]->d_inode->i_private; + securityfs_remove(profile->dents[i]); + aa_put_replacedby(r); + profile->dents[i] = NULL; + } +} + +void __aa_fs_profile_migrate_dents(struct aa_profile *old, + struct aa_profile *new) +{ + int i; + + for (i = 0; i < AAFS_PROF_SIZEOF; i++) { + new->dents[i] = old->dents[i]; + old->dents[i] = NULL; + } +} + +static struct dentry *create_profile_file(struct dentry *dir, const char *name, + struct aa_profile *profile, + const struct file_operations *fops) +{ + struct aa_replacedby *r = aa_get_replacedby(profile->replacedby); + struct dentry *dent; + + dent = securityfs_create_file(name, S_IFREG | 0444, dir, r, fops); + if (IS_ERR(dent)) + aa_put_replacedby(r); + + return dent; +} + +/* requires lock be held */ +int __aa_fs_profile_mkdir(struct aa_profile *profile, struct dentry *parent) +{ + struct aa_profile *child; + struct dentry *dent = NULL, *dir; + int error; + + if (!parent) { + struct aa_profile *p; + p = aa_deref_parent(profile); + dent = prof_dir(p); + /* adding to parent that previously didn't have children */ + dent = securityfs_create_dir("profiles", dent); + if (IS_ERR(dent)) + goto fail; + prof_child_dir(p) = parent = dent; + } + + if (!profile->dirname) { + int len, id_len; + len = mangle_name(profile->base.name, NULL); + id_len = snprintf(NULL, 0, ".%ld", profile->ns->uniq_id); + + profile->dirname = kmalloc(len + id_len + 1, GFP_KERNEL); + if (!profile->dirname) + goto fail; + + mangle_name(profile->base.name, profile->dirname); + sprintf(profile->dirname + len, ".%ld", profile->ns->uniq_id++); + } + + dent = securityfs_create_dir(profile->dirname, parent); + if (IS_ERR(dent)) + goto fail; + prof_dir(profile) = dir = dent; + + dent = create_profile_file(dir, "name", profile, &aa_fs_profname_fops); + if (IS_ERR(dent)) + goto fail; + profile->dents[AAFS_PROF_NAME] = dent; + + dent = create_profile_file(dir, "mode", profile, &aa_fs_profmode_fops); + if (IS_ERR(dent)) + goto fail; + profile->dents[AAFS_PROF_MODE] = dent; + + list_for_each_entry(child, &profile->base.profiles, base.list) { + error = __aa_fs_profile_mkdir(child, prof_child_dir(profile)); + if (error) + goto fail2; + } + + return 0; + +fail: + error = PTR_ERR(dent); + +fail2: + __aa_fs_profile_rmdir(profile); + + return error; +} + +void __aa_fs_namespace_rmdir(struct aa_namespace *ns) +{ + struct aa_namespace *sub; + struct aa_profile *child; + int i; + + if (!ns) + return; + + list_for_each_entry(child, &ns->base.profiles, base.list) + __aa_fs_profile_rmdir(child); + + list_for_each_entry(sub, &ns->sub_ns, base.list) { + mutex_lock(&sub->lock); + __aa_fs_namespace_rmdir(sub); + mutex_unlock(&sub->lock); + } + + for (i = AAFS_NS_SIZEOF - 1; i >= 0; --i) { + securityfs_remove(ns->dents[i]); + ns->dents[i] = NULL; + } +} + +int __aa_fs_namespace_mkdir(struct aa_namespace *ns, struct dentry *parent, + const char *name) +{ + struct aa_namespace *sub; + struct aa_profile *child; + struct dentry *dent, *dir; + int error; + + if (!name) + name = ns->base.name; + + dent = securityfs_create_dir(name, parent); + if (IS_ERR(dent)) + goto fail; + ns_dir(ns) = dir = dent; + + dent = securityfs_create_dir("profiles", dir); + if (IS_ERR(dent)) + goto fail; + ns_subprofs_dir(ns) = dent; + dent = securityfs_create_dir("namespaces", dir); + if (IS_ERR(dent)) + goto fail; + ns_subns_dir(ns) = dent; + + list_for_each_entry(child, &ns->base.profiles, base.list) { + error = __aa_fs_profile_mkdir(child, ns_subprofs_dir(ns)); + if (error) + goto fail2; + } + + list_for_each_entry(sub, &ns->sub_ns, base.list) { + mutex_lock(&sub->lock); + error = __aa_fs_namespace_mkdir(sub, ns_subns_dir(ns), NULL); + mutex_unlock(&sub->lock); + if (error) + goto fail2; + } + + return 0; + +fail: + error = PTR_ERR(dent); + +fail2: + __aa_fs_namespace_rmdir(ns); + + return error; +} + + +/** Base file system setup **/ static struct aa_fs_entry aa_fs_entry_file[] = { AA_FS_FILE_STRING("mask", "create read write exec append mmap_exec " \ "link lock"), @@ -246,6 +541,7 @@ static int __init aafs_create_file(struct aa_fs_entry *fs_file, return error; } +static void __init aafs_remove_dir(struct aa_fs_entry *fs_dir); /** * aafs_create_dir - recursively create a directory entry in the securityfs * @fs_dir: aa_fs_entry (and all child entries) to build (NOT NULL) @@ -256,17 +552,16 @@ static int __init aafs_create_file(struct aa_fs_entry *fs_file, static int __init aafs_create_dir(struct aa_fs_entry *fs_dir, struct dentry *parent) { - int error; struct aa_fs_entry *fs_file; + struct dentry *dir; + int error; - fs_dir->dentry = securityfs_create_dir(fs_dir->name, parent); - if (IS_ERR(fs_dir->dentry)) { - error = PTR_ERR(fs_dir->dentry); - fs_dir->dentry = NULL; - goto failed; - } + dir = securityfs_create_dir(fs_dir->name, parent); + if (IS_ERR(dir)) + return PTR_ERR(dir); + fs_dir->dentry = dir; - for (fs_file = fs_dir->v.files; fs_file->name; ++fs_file) { + for (fs_file = fs_dir->v.files; fs_file && fs_file->name; ++fs_file) { if (fs_file->v_type == AA_FS_TYPE_DIR) error = aafs_create_dir(fs_file, fs_dir->dentry); else @@ -278,6 +573,8 @@ static int __init aafs_create_dir(struct aa_fs_entry *fs_dir, return 0; failed: + aafs_remove_dir(fs_dir); + return error; } @@ -302,7 +599,7 @@ static void __init aafs_remove_dir(struct aa_fs_entry *fs_dir) { struct aa_fs_entry *fs_file; - for (fs_file = fs_dir->v.files; fs_file->name; ++fs_file) { + for (fs_file = fs_dir->v.files; fs_file && fs_file->name; ++fs_file) { if (fs_file->v_type == AA_FS_TYPE_DIR) aafs_remove_dir(fs_file); else @@ -346,6 +643,11 @@ static int __init aa_create_aafs(void) if (error) goto error; + error = __aa_fs_namespace_mkdir(root_ns, aa_fs_entry.dentry, + "policy"); + if (error) + goto error; + /* TODO: add support for apparmorfs_null and apparmorfs_mnt */ /* Report that AppArmor fs is enabled */ diff --git a/security/apparmor/include/apparmorfs.h b/security/apparmor/include/apparmorfs.h index 7ea4769fab3f..2494e112f2bf 100644 --- a/security/apparmor/include/apparmorfs.h +++ b/security/apparmor/include/apparmorfs.h @@ -61,4 +61,42 @@ extern const struct file_operations aa_fs_seq_file_ops; extern void __init aa_destroy_aafs(void); +struct aa_profile; +struct aa_namespace; + +enum aafs_ns_type { + AAFS_NS_DIR, + AAFS_NS_PROFS, + AAFS_NS_NS, + AAFS_NS_COUNT, + AAFS_NS_MAX_COUNT, + AAFS_NS_SIZE, + AAFS_NS_MAX_SIZE, + AAFS_NS_OWNER, + AAFS_NS_SIZEOF, +}; + +enum aafs_prof_type { + AAFS_PROF_DIR, + AAFS_PROF_PROFS, + AAFS_PROF_NAME, + AAFS_PROF_MODE, + AAFS_PROF_SIZEOF, +}; + +#define ns_dir(X) ((X)->dents[AAFS_NS_DIR]) +#define ns_subns_dir(X) ((X)->dents[AAFS_NS_NS]) +#define ns_subprofs_dir(X) ((X)->dents[AAFS_NS_PROFS]) + +#define prof_dir(X) ((X)->dents[AAFS_PROF_DIR]) +#define prof_child_dir(X) ((X)->dents[AAFS_PROF_PROFS]) + +void __aa_fs_profile_rmdir(struct aa_profile *profile); +void __aa_fs_profile_migrate_dents(struct aa_profile *old, + struct aa_profile *new); +int __aa_fs_profile_mkdir(struct aa_profile *profile, struct dentry *parent); +void __aa_fs_namespace_rmdir(struct aa_namespace *ns); +int __aa_fs_namespace_mkdir(struct aa_namespace *ns, struct dentry *parent, + const char *name); + #endif /* __AA_APPARMORFS_H */ diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h index 69d8cae634e7..30e8d7687259 100644 --- a/security/apparmor/include/audit.h +++ b/security/apparmor/include/audit.h @@ -27,7 +27,6 @@ struct aa_profile; extern const char *const audit_mode_names[]; #define AUDIT_MAX_INDEX 5 - enum audit_mode { AUDIT_NORMAL, /* follow normal auditing of accesses */ AUDIT_QUIET_DENIED, /* quiet all denied access messages */ diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 65662e3c75cf..5c72231d1c42 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -29,8 +29,8 @@ #include "file.h" #include "resource.h" -extern const char *const profile_mode_names[]; -#define APPARMOR_NAMES_MAX_INDEX 3 +extern const char *const aa_profile_mode_names[]; +#define APPARMOR_MODE_NAMES_MAX_INDEX 4 #define PROFILE_MODE(_profile, _mode) \ ((aa_g_profile_mode == (_mode)) || \ @@ -110,6 +110,8 @@ struct aa_ns_acct { * @unconfined: special unconfined profile for the namespace * @sub_ns: list of namespaces under the current namespace. * @uniq_null: uniq value used for null learning profiles + * @uniq_id: a unique id count for the profiles in the namespace + * @dents: dentries for the namespaces file entries in apparmorfs * * An aa_namespace defines the set profiles that are searched to determine * which profile to attach to a task. Profiles can not be shared between @@ -133,6 +135,9 @@ struct aa_namespace { struct aa_profile *unconfined; struct list_head sub_ns; atomic_t uniq_null; + long uniq_id; + + struct dentry *dents[AAFS_NS_SIZEOF]; }; /* struct aa_policydb - match engine for a policy @@ -172,6 +177,9 @@ struct aa_replacedby { * @caps: capabilities for the profile * @rlimits: rlimits for the profile * + * @dents: dentries for the profiles file entries in apparmorfs + * @dirname: name of the profile dir in apparmorfs + * * The AppArmor profile contains the basic confinement data. Each profile * has a name, and exists in a namespace. The @name and @exec_match are * used to determine profile attachment against unconfined tasks. All other @@ -208,6 +216,9 @@ struct aa_profile { struct aa_file_rules file; struct aa_caps caps; struct aa_rlimit rlimits; + + char *dirname; + struct dentry *dents[AAFS_PROF_SIZEOF]; }; extern struct aa_namespace *root_ns; @@ -243,6 +254,12 @@ ssize_t aa_remove_profiles(char *name, size_t size); #define unconfined(X) ((X)->mode == APPARMOR_UNCONFINED) +static inline struct aa_profile *aa_deref_parent(struct aa_profile *p) +{ + return rcu_dereference_protected(p->parent, + mutex_is_locked(&p->ns->lock)); +} + /** * aa_get_profile - increment refcount on profile @p * @p: profile (MAYBE NULL) diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index c8c148a738f7..edb3ce15e92d 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -843,7 +843,7 @@ static int param_get_mode(char *buffer, struct kernel_param *kp) if (!apparmor_enabled) return -EINVAL; - return sprintf(buffer, "%s", profile_mode_names[aa_g_profile_mode]); + return sprintf(buffer, "%s", aa_profile_mode_names[aa_g_profile_mode]); } static int param_set_mode(const char *val, struct kernel_param *kp) @@ -858,8 +858,8 @@ static int param_set_mode(const char *val, struct kernel_param *kp) if (!val) return -EINVAL; - for (i = 0; i < APPARMOR_NAMES_MAX_INDEX; i++) { - if (strcmp(val, profile_mode_names[i]) == 0) { + for (i = 0; i < APPARMOR_MODE_NAMES_MAX_INDEX; i++) { + if (strcmp(val, aa_profile_mode_names[i]) == 0) { aa_g_profile_mode = i; return 0; } diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 2e4e2ecb25bc..6172509fa2b7 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -92,7 +92,7 @@ /* root profile namespace */ struct aa_namespace *root_ns; -const char *const profile_mode_names[] = { +const char *const aa_profile_mode_names[] = { "enforce", "complain", "kill", @@ -394,7 +394,13 @@ static struct aa_namespace *aa_prepare_namespace(const char *name) ns = alloc_namespace(root->base.hname, name); if (!ns) goto out; - /* add parent ref */ + if (__aa_fs_namespace_mkdir(ns, ns_subns_dir(root), name)) { + AA_ERROR("Failed to create interface for ns %s\n", + ns->base.name); + free_namespace(ns); + ns = NULL; + goto out; + } ns->parent = aa_get_namespace(root); list_add_rcu(&ns->base.list, &root->sub_ns); /* add list ref */ @@ -456,6 +462,7 @@ static void __remove_profile(struct aa_profile *profile) __profile_list_release(&profile->base.profiles); /* released by free_profile */ __aa_update_replacedby(profile, profile->ns->unconfined); + __aa_fs_profile_rmdir(profile); __list_remove_profile(profile); } @@ -492,6 +499,7 @@ static void destroy_namespace(struct aa_namespace *ns) if (ns->parent) __aa_update_replacedby(ns->unconfined, ns->parent->unconfined); + __aa_fs_namespace_rmdir(ns); mutex_unlock(&ns->lock); } @@ -596,6 +604,7 @@ void aa_free_profile(struct aa_profile *profile) aa_free_cap_rules(&profile->caps); aa_free_rlimit_rules(&profile->rlimits); + kzfree(profile->dirname); aa_put_dfa(profile->xmatch); aa_put_dfa(profile->policy.dfa); aa_put_replacedby(profile->replacedby); @@ -986,8 +995,7 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new, /* inherit @child and its children */ /* TODO: update hname of inherited children */ /* list refcount transferred to @new */ - p = rcu_dereference_protected(child->parent, - mutex_is_locked(&child->ns->lock)); + p = aa_deref_parent(child); rcu_assign_pointer(child->parent, aa_get_profile(new)); list_add_rcu(&child->base.list, &new->base.profiles); aa_put_profile(p); @@ -995,14 +1003,18 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new, } if (!rcu_access_pointer(new->parent)) { - struct aa_profile *parent = rcu_dereference(old->parent); + struct aa_profile *parent = aa_deref_parent(old); rcu_assign_pointer(new->parent, aa_get_profile(parent)); } __aa_update_replacedby(old, new); if (share_replacedby) { aa_put_replacedby(new->replacedby); new->replacedby = aa_get_replacedby(old->replacedby); - } + } else if (!rcu_access_pointer(new->replacedby->profile)) + /* aafs interface uses replacedby */ + rcu_assign_pointer(new->replacedby->profile, + aa_get_profile(new)); + __aa_fs_profile_migrate_dents(old, new); if (list_empty(&new->base.list)) { /* new is not on a list already */ @@ -1118,7 +1130,33 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) } } - /* do actual replacement */ + /* create new fs entries for introspection if needed */ + list_for_each_entry(ent, &lh, list) { + if (ent->old) { + /* inherit old interface files */ + + /* if (ent->rename) + TODO: support rename */ + /* } else if (ent->rename) { + TODO: support rename */ + } else { + struct dentry *parent; + if (rcu_access_pointer(ent->new->parent)) { + struct aa_profile *p; + p = aa_deref_parent(ent->new); + parent = prof_child_dir(p); + } else + parent = ns_subprofs_dir(ent->new->ns); + error = __aa_fs_profile_mkdir(ent->new, parent); + } + + if (error) { + info = "failed to create "; + goto fail_lock; + } + } + + /* Done with checks that may fail - do actual replacement */ list_for_each_entry_safe(ent, tmp, &lh, list) { list_del_init(&ent->list); op = (!ent->old && !ent->rename) ? OP_PROF_LOAD : OP_PROF_REPL; @@ -1127,14 +1165,21 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) if (ent->old) { __replace_profile(ent->old, ent->new, 1); - if (ent->rename) + if (ent->rename) { + /* aafs interface uses replacedby */ + struct aa_replacedby *r = ent->new->replacedby; + rcu_assign_pointer(r->profile, + aa_get_profile(ent->new)); __replace_profile(ent->rename, ent->new, 0); + } } else if (ent->rename) { + /* aafs interface uses replacedby */ + rcu_assign_pointer(ent->new->replacedby->profile, + aa_get_profile(ent->new)); __replace_profile(ent->rename, ent->new, 0); } else if (ent->new->parent) { struct aa_profile *parent, *newest; - parent = rcu_dereference_protected(ent->new->parent, - mutex_is_locked(&ns->lock)); + parent = aa_deref_parent(ent->new); newest = aa_get_newest_profile(parent); /* parent replaced in this atomic set? */ @@ -1144,10 +1189,16 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) rcu_assign_pointer(ent->new->parent, newest); } else aa_put_profile(newest); + /* aafs interface uses replacedby */ + rcu_assign_pointer(ent->new->replacedby->profile, + aa_get_profile(ent->new)); __list_add_profile(&parent->base.profiles, ent->new); - } else + } else { + /* aafs interface uses replacedby */ + rcu_assign_pointer(ent->new->replacedby->profile, + aa_get_profile(ent->new)); __list_add_profile(&ns->base.profiles, ent->new); - + } aa_load_ent_free(ent); } mutex_unlock(&ns->lock); diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c index 6c9390179b89..b125acc9aa26 100644 --- a/security/apparmor/procattr.c +++ b/security/apparmor/procattr.c @@ -37,7 +37,7 @@ int aa_getprocattr(struct aa_profile *profile, char **string) { char *str; int len = 0, mode_len = 0, ns_len = 0, name_len; - const char *mode_str = profile_mode_names[profile->mode]; + const char *mode_str = aa_profile_mode_names[profile->mode]; const char *ns_name = NULL; struct aa_namespace *ns = profile->ns; struct aa_namespace *current_ns = __aa_current_profile()->ns; -- cgit v1.2.3 From 556d0be74b19cb6288e5eb2f3216eac247d87968 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:17:43 -0700 Subject: apparmor: add an optional profile attachment string for profiles 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 Acked-by: Seth Arnold --- security/apparmor/apparmorfs.c | 34 ++++++++++++++++++++++++++++++++++ security/apparmor/include/apparmorfs.h | 1 + security/apparmor/include/policy.h | 2 ++ security/apparmor/policy_unpack.c | 3 +++ 4 files changed, 40 insertions(+) (limited to 'security') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 0fdd08c6ea59..d6329aa7aa98 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -290,6 +290,34 @@ static const struct file_operations aa_fs_profmode_fops = { .release = aa_fs_seq_profile_release, }; +static int aa_fs_seq_profattach_show(struct seq_file *seq, void *v) +{ + struct aa_replacedby *r = seq->private; + struct aa_profile *profile = aa_get_profile_rcu(&r->profile); + if (profile->attach) + seq_printf(seq, "%s\n", profile->attach); + else if (profile->xmatch) + seq_puts(seq, "\n"); + else + seq_printf(seq, "%s\n", profile->base.name); + aa_put_profile(profile); + + return 0; +} + +static int aa_fs_seq_profattach_open(struct inode *inode, struct file *file) +{ + return aa_fs_seq_profile_open(inode, file, aa_fs_seq_profattach_show); +} + +static const struct file_operations aa_fs_profattach_fops = { + .owner = THIS_MODULE, + .open = aa_fs_seq_profattach_open, + .read = seq_read, + .llseek = seq_lseek, + .release = aa_fs_seq_profile_release, +}; + /** fns to setup dynamic per profile/namespace files **/ void __aa_fs_profile_rmdir(struct aa_profile *profile) { @@ -385,6 +413,12 @@ int __aa_fs_profile_mkdir(struct aa_profile *profile, struct dentry *parent) goto fail; profile->dents[AAFS_PROF_MODE] = dent; + dent = create_profile_file(dir, "attach", profile, + &aa_fs_profattach_fops); + if (IS_ERR(dent)) + goto fail; + profile->dents[AAFS_PROF_ATTACH] = dent; + list_for_each_entry(child, &profile->base.profiles, base.list) { error = __aa_fs_profile_mkdir(child, prof_child_dir(profile)); if (error) diff --git a/security/apparmor/include/apparmorfs.h b/security/apparmor/include/apparmorfs.h index 2494e112f2bf..f91712cf1b30 100644 --- a/security/apparmor/include/apparmorfs.h +++ b/security/apparmor/include/apparmorfs.h @@ -81,6 +81,7 @@ enum aafs_prof_type { AAFS_PROF_PROFS, AAFS_PROF_NAME, AAFS_PROF_MODE, + AAFS_PROF_ATTACH, AAFS_PROF_SIZEOF, }; diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 5c72231d1c42..59b36372ae40 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -165,6 +165,7 @@ struct aa_replacedby { * @ns: namespace the profile is in * @replacedby: is set to the profile that replaced this profile * @rename: optional profile name that this profile renamed + * @attach: human readable attachment string * @xmatch: optional extended matching for unconfined executables names * @xmatch_len: xmatch prefix len, used to determine xmatch priority * @audit: the auditing mode of the profile @@ -204,6 +205,7 @@ struct aa_profile { struct aa_replacedby *replacedby; const char *rename; + const char *attach; struct aa_dfa *xmatch; int xmatch_len; enum audit_mode audit; diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index cac0aa075787..bdaef2e1b2a0 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -492,6 +492,9 @@ static struct aa_profile *unpack_profile(struct aa_ext *e) /* profile renaming is optional */ (void) unpack_str(e, &profile->rename, "rename"); + /* attachment string is optional */ + (void) unpack_str(e, &profile->attach, "attach"); + /* xmatch is optional and may be NULL */ profile->xmatch = unpack_dfa(e); if (IS_ERR(profile->xmatch)) { -- cgit v1.2.3 From 29b3822f1e132aa0f115f69730d6e4182df153d4 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 10 Jul 2013 21:18:43 -0700 Subject: apparmor: add the profile introspection file to interface Add the dynamic namespace relative profiles file to the interace, to allow introspection of loaded profiles and their modes. Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/apparmorfs.c | 236 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 236 insertions(+) (limited to 'security') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index d6329aa7aa98..7a26608a5666 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "include/apparmor.h" #include "include/apparmorfs.h" @@ -512,6 +513,240 @@ fail2: } +#define list_entry_next(pos, member) \ + list_entry(pos->member.next, typeof(*pos), member) +#define list_entry_is_head(pos, head, member) (&pos->member == (head)) + +/** + * __next_namespace - find the next namespace to list + * @root: root namespace to stop search at (NOT NULL) + * @ns: current ns position (NOT NULL) + * + * Find the next namespace from @ns under @root and handle all locking needed + * while switching current namespace. + * + * Returns: next namespace or NULL if at last namespace under @root + * Requires: ns->parent->lock to be held + * NOTE: will not unlock root->lock + */ +static struct aa_namespace *__next_namespace(struct aa_namespace *root, + struct aa_namespace *ns) +{ + struct aa_namespace *parent, *next; + + /* is next namespace a child */ + if (!list_empty(&ns->sub_ns)) { + next = list_first_entry(&ns->sub_ns, typeof(*ns), base.list); + mutex_lock(&next->lock); + return next; + } + + /* check if the next ns is a sibling, parent, gp, .. */ + parent = ns->parent; + while (parent) { + 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; + } + + return NULL; +} + +/** + * __first_profile - find the first profile in a namespace + * @root: namespace that is root of profiles being displayed (NOT NULL) + * @ns: namespace to start in (NOT NULL) + * + * Returns: unrefcounted profile or NULL if no profile + * Requires: profile->ns.lock to be held + */ +static struct aa_profile *__first_profile(struct aa_namespace *root, + struct aa_namespace *ns) +{ + for (; ns; ns = __next_namespace(root, ns)) { + if (!list_empty(&ns->base.profiles)) + return list_first_entry(&ns->base.profiles, + struct aa_profile, base.list); + } + return NULL; +} + +/** + * __next_profile - step to the next profile in a profile tree + * @profile: current profile in tree (NOT NULL) + * + * Perform a depth first traversal on the profile tree in a namespace + * + * Returns: next profile or NULL if done + * Requires: profile->ns.lock to be held + */ +static struct aa_profile *__next_profile(struct aa_profile *p) +{ + struct aa_profile *parent; + struct aa_namespace *ns = p->ns; + + /* is next profile a child */ + if (!list_empty(&p->base.profiles)) + return list_first_entry(&p->base.profiles, typeof(*p), + base.list); + + /* is next profile a sibling, parent sibling, gp, sibling, .. */ + parent = rcu_dereference_protected(p->parent, + mutex_is_locked(&p->ns->lock)); + while (parent) { + p = list_entry_next(p, base.list); + if (!list_entry_is_head(p, &parent->base.profiles, base.list)) + return p; + p = parent; + parent = rcu_dereference_protected(parent->parent, + mutex_is_locked(&parent->ns->lock)); + } + + /* is next another profile in the namespace */ + p = list_entry_next(p, base.list); + if (!list_entry_is_head(p, &ns->base.profiles, base.list)) + return p; + + return NULL; +} + +/** + * next_profile - step to the next profile in where ever it may be + * @root: root namespace (NOT NULL) + * @profile: current profile (NOT NULL) + * + * Returns: next profile or NULL if there isn't one + */ +static struct aa_profile *next_profile(struct aa_namespace *root, + struct aa_profile *profile) +{ + struct aa_profile *next = __next_profile(profile); + if (next) + return next; + + /* finished all profiles in namespace move to next namespace */ + return __first_profile(root, __next_namespace(root, profile->ns)); +} + +/** + * p_start - start a depth first traversal of profile tree + * @f: seq_file to fill + * @pos: current position + * + * Returns: first profile under current namespace or NULL if none found + * + * acquires first ns->lock + */ +static void *p_start(struct seq_file *f, loff_t *pos) +{ + struct aa_profile *profile = NULL; + struct aa_namespace *root = aa_current_profile()->ns; + loff_t l = *pos; + f->private = aa_get_namespace(root); + + + /* find the first profile */ + mutex_lock(&root->lock); + profile = __first_profile(root, root); + + /* skip to position */ + for (; profile && l > 0; l--) + profile = next_profile(root, profile); + + return profile; +} + +/** + * p_next - read the next profile entry + * @f: seq_file to fill + * @p: profile previously returned + * @pos: current position + * + * Returns: next profile after @p or NULL if none + * + * may acquire/release locks in namespace tree as necessary + */ +static void *p_next(struct seq_file *f, void *p, loff_t *pos) +{ + struct aa_profile *profile = p; + struct aa_namespace *ns = f->private; + (*pos)++; + + return next_profile(ns, profile); +} + +/** + * p_stop - stop depth first traversal + * @f: seq_file we are filling + * @p: the last profile writen + * + * Release all locking done by p_start/p_next on namespace tree + */ +static void p_stop(struct seq_file *f, void *p) +{ + struct aa_profile *profile = p; + struct aa_namespace *root = f->private, *ns; + + if (profile) { + for (ns = profile->ns; ns && ns != root; ns = ns->parent) + mutex_unlock(&ns->lock); + } + mutex_unlock(&root->lock); + aa_put_namespace(root); +} + +/** + * seq_show_profile - show a profile entry + * @f: seq_file to file + * @p: current position (profile) (NOT NULL) + * + * Returns: error on failure + */ +static int seq_show_profile(struct seq_file *f, void *p) +{ + struct aa_profile *profile = (struct aa_profile *)p; + struct aa_namespace *root = f->private; + + if (profile->ns != root) + seq_printf(f, ":%s://", aa_ns_name(root, profile->ns)); + seq_printf(f, "%s (%s)\n", profile->base.hname, + aa_profile_mode_names[profile->mode]); + + return 0; +} + +static const struct seq_operations aa_fs_profiles_op = { + .start = p_start, + .next = p_next, + .stop = p_stop, + .show = seq_show_profile, +}; + +static int profiles_open(struct inode *inode, struct file *file) +{ + return seq_open(file, &aa_fs_profiles_op); +} + +static int profiles_release(struct inode *inode, struct file *file) +{ + return seq_release(inode, file); +} + +static const struct file_operations aa_fs_profiles_fops = { + .open = profiles_open, + .read = seq_read, + .llseek = seq_lseek, + .release = profiles_release, +}; + + /** Base file system setup **/ static struct aa_fs_entry aa_fs_entry_file[] = { AA_FS_FILE_STRING("mask", "create read write exec append mmap_exec " \ @@ -545,6 +780,7 @@ static struct aa_fs_entry aa_fs_entry_apparmor[] = { AA_FS_FILE_FOPS(".load", 0640, &aa_fs_profile_load), AA_FS_FILE_FOPS(".replace", 0640, &aa_fs_profile_replace), AA_FS_FILE_FOPS(".remove", 0640, &aa_fs_profile_remove), + AA_FS_FILE_FOPS("profiles", 0640, &aa_fs_profiles_fops), AA_FS_DIR("features", aa_fs_entry_features), { } }; -- cgit v1.2.3 From 84f1f787421cd83bb7dfb34d584586f6a5fe7baa Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 14 Aug 2013 11:27:32 -0700 Subject: apparmor: export set of capabilities supported by the apparmor module Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/Makefile | 6 +++++- security/apparmor/apparmorfs.c | 1 + security/apparmor/capability.c | 5 +++++ security/apparmor/include/capability.h | 4 ++++ 4 files changed, 15 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/Makefile b/security/apparmor/Makefile index 5706b74c857f..0831e049072d 100644 --- a/security/apparmor/Makefile +++ b/security/apparmor/Makefile @@ -18,7 +18,11 @@ quiet_cmd_make-caps = GEN $@ cmd_make-caps = echo "static const char *const capability_names[] = {" > $@ ;\ sed $< >>$@ -r -n -e '/CAP_FS_MASK/d' \ -e 's/^\#define[ \t]+CAP_([A-Z0-9_]+)[ \t]+([0-9]+)/[\2] = "\L\1",/p';\ - echo "};" >> $@ + echo "};" >> $@ ;\ + echo -n '\#define AA_FS_CAPS_MASK "' >> $@ ;\ + sed $< -r -n -e '/CAP_FS_MASK/d' \ + -e 's/^\#define[ \t]+CAP_([A-Z0-9_]+)[ \t]+([0-9]+)/\L\1/p' | \ + tr '\n' ' ' | sed -e 's/ $$/"\n/' >> $@ # Build a lower case string table of rlimit names. diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 7a26608a5666..d708a55d072f 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -773,6 +773,7 @@ static struct aa_fs_entry aa_fs_entry_features[] = { AA_FS_DIR("file", aa_fs_entry_file), AA_FS_FILE_U64("capability", VFS_CAP_FLAGS_MASK), AA_FS_DIR("rlimit", aa_fs_entry_rlimit), + AA_FS_DIR("caps", aa_fs_entry_caps), { } }; diff --git a/security/apparmor/capability.c b/security/apparmor/capability.c index 887a5e948945..84d1f5f53877 100644 --- a/security/apparmor/capability.c +++ b/security/apparmor/capability.c @@ -27,6 +27,11 @@ */ #include "capability_names.h" +struct aa_fs_entry aa_fs_entry_caps[] = { + AA_FS_FILE_STRING("mask", AA_FS_CAPS_MASK), + { } +}; + struct audit_cache { struct aa_profile *profile; kernel_cap_t caps; diff --git a/security/apparmor/include/capability.h b/security/apparmor/include/capability.h index c24d2959ea02..2e7c9d6a2f3b 100644 --- a/security/apparmor/include/capability.h +++ b/security/apparmor/include/capability.h @@ -17,6 +17,8 @@ #include +#include "apparmorfs.h" + struct aa_profile; /* aa_caps - confinement data for capabilities @@ -34,6 +36,8 @@ struct aa_caps { kernel_cap_t extended; }; +extern struct aa_fs_entry aa_fs_entry_caps[]; + int aa_capable(struct task_struct *task, struct aa_profile *profile, int cap, int audit); -- cgit v1.2.3 From f8eb8a1324e81927b2c64823b2fc38386efd3fef Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 14 Aug 2013 11:27:36 -0700 Subject: apparmor: add the ability to report a sha1 hash of loaded policy Provide userspace the ability to introspect a sha1 hash value for each profile currently loaded. Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/Kconfig | 12 +++++ security/apparmor/Makefile | 1 + security/apparmor/apparmorfs.c | 37 +++++++++++++ security/apparmor/crypto.c | 97 ++++++++++++++++++++++++++++++++++ security/apparmor/include/apparmorfs.h | 1 + security/apparmor/include/crypto.h | 36 +++++++++++++ security/apparmor/include/policy.h | 1 + security/apparmor/policy_unpack.c | 20 ++++--- 8 files changed, 199 insertions(+), 6 deletions(-) create mode 100644 security/apparmor/crypto.c create mode 100644 security/apparmor/include/crypto.h (limited to 'security') diff --git a/security/apparmor/Kconfig b/security/apparmor/Kconfig index 9b9013b2e321..d49c53960b60 100644 --- a/security/apparmor/Kconfig +++ b/security/apparmor/Kconfig @@ -29,3 +29,15 @@ config SECURITY_APPARMOR_BOOTPARAM_VALUE boot. If you are unsure how to answer this question, answer 1. + +config SECURITY_APPARMOR_HASH + bool "SHA1 hash of loaded profiles" + depends on SECURITY_APPARMOR + depends on CRYPTO + select CRYPTO_SHA1 + default y + + help + This option selects whether sha1 hashing is done against loaded + profiles and exported for inspection to user space via the apparmor + filesystem. diff --git a/security/apparmor/Makefile b/security/apparmor/Makefile index 0831e049072d..d693df874818 100644 --- a/security/apparmor/Makefile +++ b/security/apparmor/Makefile @@ -5,6 +5,7 @@ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor.o apparmor-y := apparmorfs.o audit.o capability.o context.o ipc.o lib.o match.o \ path.o domain.o policy.o policy_unpack.o procattr.o lsm.o \ resource.o sid.o file.o +apparmor-$(CONFIG_SECURITY_APPARMOR_HASH) += crypto.o clean-files := capability_names.h rlim_names.h diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index d708a55d072f..95c2b2689a03 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -26,6 +26,7 @@ #include "include/apparmorfs.h" #include "include/audit.h" #include "include/context.h" +#include "include/crypto.h" #include "include/policy.h" #include "include/resource.h" @@ -319,6 +320,34 @@ static const struct file_operations aa_fs_profattach_fops = { .release = aa_fs_seq_profile_release, }; +static int aa_fs_seq_hash_show(struct seq_file *seq, void *v) +{ + struct aa_replacedby *r = seq->private; + struct aa_profile *profile = aa_get_profile_rcu(&r->profile); + unsigned int i, size = aa_hash_size(); + + if (profile->hash) { + for (i = 0; i < size; i++) + seq_printf(seq, "%.2x", profile->hash[i]); + seq_puts(seq, "\n"); + } + + return 0; +} + +static int aa_fs_seq_hash_open(struct inode *inode, struct file *file) +{ + return single_open(file, aa_fs_seq_hash_show, inode->i_private); +} + +static const struct file_operations aa_fs_seq_hash_fops = { + .owner = THIS_MODULE, + .open = aa_fs_seq_hash_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + /** fns to setup dynamic per profile/namespace files **/ void __aa_fs_profile_rmdir(struct aa_profile *profile) { @@ -420,6 +449,14 @@ int __aa_fs_profile_mkdir(struct aa_profile *profile, struct dentry *parent) goto fail; profile->dents[AAFS_PROF_ATTACH] = dent; + if (profile->hash) { + dent = create_profile_file(dir, "sha1", profile, + &aa_fs_seq_hash_fops); + if (IS_ERR(dent)) + goto fail; + profile->dents[AAFS_PROF_HASH] = dent; + } + list_for_each_entry(child, &profile->base.profiles, base.list) { error = __aa_fs_profile_mkdir(child, prof_child_dir(profile)); if (error) diff --git a/security/apparmor/crypto.c b/security/apparmor/crypto.c new file mode 100644 index 000000000000..d6222ba4e919 --- /dev/null +++ b/security/apparmor/crypto.c @@ -0,0 +1,97 @@ +/* + * AppArmor security module + * + * This file contains AppArmor policy loading interface function definitions. + * + * Copyright 2013 Canonical Ltd. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, version 2 of the + * License. + * + * Fns to provide a checksum of policy that has been loaded this can be + * compared to userspace policy compiles to check loaded policy is what + * it should be. + */ + +#include + +#include "include/apparmor.h" +#include "include/crypto.h" + +static unsigned int apparmor_hash_size; + +static struct crypto_hash *apparmor_tfm; + +unsigned int aa_hash_size(void) +{ + return apparmor_hash_size; +} + +int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start, + size_t len) +{ + struct scatterlist sg[2]; + struct hash_desc desc = { + .tfm = apparmor_tfm, + .flags = 0 + }; + int error = -ENOMEM; + u32 le32_version = cpu_to_le32(version); + + if (!apparmor_tfm) + return 0; + + sg_init_table(sg, 2); + sg_set_buf(&sg[0], &le32_version, 4); + sg_set_buf(&sg[1], (u8 *) start, len); + + profile->hash = kzalloc(apparmor_hash_size, GFP_KERNEL); + if (!profile->hash) + goto fail; + + error = crypto_hash_init(&desc); + if (error) + goto fail; + error = crypto_hash_update(&desc, &sg[0], 4); + if (error) + goto fail; + error = crypto_hash_update(&desc, &sg[1], len); + if (error) + goto fail; + error = crypto_hash_final(&desc, profile->hash); + if (error) + goto fail; + + return 0; + +fail: + kfree(profile->hash); + profile->hash = NULL; + + return error; +} + +static int __init init_profile_hash(void) +{ + struct crypto_hash *tfm; + + if (!apparmor_initialized) + return 0; + + tfm = crypto_alloc_hash("sha1", 0, CRYPTO_ALG_ASYNC); + if (IS_ERR(tfm)) { + int error = PTR_ERR(tfm); + AA_ERROR("failed to setup profile sha1 hashing: %d\n", error); + return error; + } + apparmor_tfm = tfm; + apparmor_hash_size = crypto_hash_digestsize(apparmor_tfm); + + aa_info_message("AppArmor sha1 policy hashing enabled"); + + return 0; +} + +late_initcall(init_profile_hash); diff --git a/security/apparmor/include/apparmorfs.h b/security/apparmor/include/apparmorfs.h index f91712cf1b30..414e56878dd0 100644 --- a/security/apparmor/include/apparmorfs.h +++ b/security/apparmor/include/apparmorfs.h @@ -82,6 +82,7 @@ enum aafs_prof_type { AAFS_PROF_NAME, AAFS_PROF_MODE, AAFS_PROF_ATTACH, + AAFS_PROF_HASH, AAFS_PROF_SIZEOF, }; diff --git a/security/apparmor/include/crypto.h b/security/apparmor/include/crypto.h new file mode 100644 index 000000000000..dc418e5024d9 --- /dev/null +++ b/security/apparmor/include/crypto.h @@ -0,0 +1,36 @@ +/* + * AppArmor security module + * + * This file contains AppArmor policy loading interface function definitions. + * + * Copyright 2013 Canonical Ltd. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, version 2 of the + * License. + */ + +#ifndef __APPARMOR_CRYPTO_H +#define __APPARMOR_CRYPTO_H + +#include "policy.h" + +#ifdef CONFIG_SECURITY_APPARMOR_HASH +unsigned int aa_hash_size(void); +int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start, + size_t len); +#else +static inline int aa_calc_profile_hash(struct aa_profile *profile, u32 version, + void *start, size_t len) +{ + return 0; +} + +static inline unsigned int aa_hash_size(void) +{ + return 0; +} +#endif + +#endif /* __APPARMOR_CRYPTO_H */ diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 59b36372ae40..f2d4b6348cbc 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -219,6 +219,7 @@ struct aa_profile { struct aa_caps caps; struct aa_rlimit rlimits; + unsigned char *hash; char *dirname; struct dentry *dents[AAFS_PROF_SIZEOF]; }; diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index bdaef2e1b2a0..a689f10930b5 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -24,6 +24,7 @@ #include "include/apparmor.h" #include "include/audit.h" #include "include/context.h" +#include "include/crypto.h" #include "include/match.h" #include "include/policy.h" #include "include/policy_unpack.h" @@ -758,10 +759,12 @@ int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns) *ns = NULL; while (e.pos < e.end) { + void *start; error = verify_header(&e, e.pos == e.start, ns); if (error) goto fail; + start = e.pos; profile = unpack_profile(&e); if (IS_ERR(profile)) { error = PTR_ERR(profile); @@ -769,16 +772,18 @@ int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns) } error = verify_profile(profile); - if (error) { - aa_free_profile(profile); - goto fail; - } + if (error) + goto fail_profile; + + error = aa_calc_profile_hash(profile, e.version, start, + e.pos - start); + if (error) + goto fail_profile; ent = aa_load_ent_alloc(); if (!ent) { error = -ENOMEM; - aa_put_profile(profile); - goto fail; + goto fail_profile; } ent->new = profile; @@ -787,6 +792,9 @@ int aa_unpack(void *udata, size_t size, struct list_head *lh, const char **ns) return 0; +fail_profile: + aa_put_profile(profile); + fail: list_for_each_entry_safe(ent, tmp, lh, list) { list_del_init(&ent->list); -- cgit v1.2.3 From 5265fc6219ddbf8dfc9b18223448a4997fb06eae Mon Sep 17 00:00:00 2001 From: Steven Rostedt Date: Tue, 20 Aug 2013 15:33:20 +0930 Subject: module/lsm: Have apparmor module parameters work with no args 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 Signed-off-by: Steven Rostedt Signed-off-by: Rusty Russell --- security/apparmor/lsm.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'security') diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 2e2a0dd4a73f..e3a704c75ef6 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -666,6 +666,7 @@ static int param_set_aabool(const char *val, const struct kernel_param *kp); static int param_get_aabool(char *buffer, const struct kernel_param *kp); #define param_check_aabool param_check_bool static struct kernel_param_ops param_ops_aabool = { + .flags = KERNEL_PARAM_FL_NOARG, .set = param_set_aabool, .get = param_get_aabool }; @@ -682,6 +683,7 @@ static int param_set_aalockpolicy(const char *val, const struct kernel_param *kp static int param_get_aalockpolicy(char *buffer, const struct kernel_param *kp); #define param_check_aalockpolicy param_check_bool static struct kernel_param_ops param_ops_aalockpolicy = { + .flags = KERNEL_PARAM_FL_NOARG, .set = param_set_aalockpolicy, .get = param_get_aalockpolicy }; -- cgit v1.2.3 From 160da84dbb39443fdade7151bc63a88f8e953077 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Tue, 2 Jul 2013 10:04:54 -0700 Subject: userns: Allow PR_CAPBSET_DROP in a user namespace. As the capabilites and capability bounding set are per user namespace properties it is safe to allow changing them with just CAP_SETPCAP permission in the user namespace. Acked-by: Serge Hallyn Tested-by: Richard Weinberger Signed-off-by: "Eric W. Biederman" --- security/commoncap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index c44b6fe6648e..9fccf71b2b62 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -824,7 +824,7 @@ int cap_task_setnice(struct task_struct *p, int nice) */ static long cap_prctl_drop(struct cred *new, unsigned long cap) { - if (!capable(CAP_SETPCAP)) + if (!ns_capable(current_user_ns(), CAP_SETPCAP)) return -EPERM; if (!cap_valid(cap)) return -EINVAL; -- cgit v1.2.3 From f54fb863c6bbcbafdfc332b4a4260abb5a002137 Mon Sep 17 00:00:00 2001 From: Serge Hallyn Date: Tue, 23 Jul 2013 13:18:53 -0500 Subject: capabilities: allow nice if we are privileged We allow task A to change B's nice level if it has a supserset of B's privileges, or of it has CAP_SYS_NICE. Also allow it if A has CAP_SYS_NICE with respect to B - meaning it is root in the same namespace, or it created B's namespace. Signed-off-by: Serge Hallyn Reviewed-by: "Eric W. Biederman" Signed-off-by: Eric W. Biederman --- security/commoncap.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/commoncap.c b/security/commoncap.c index 9fccf71b2b62..b9d613e0ef14 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -768,16 +768,16 @@ int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags) */ static int cap_safe_nice(struct task_struct *p) { - int is_subset; + int is_subset, ret = 0; rcu_read_lock(); is_subset = cap_issubset(__task_cred(p)->cap_permitted, current_cred()->cap_permitted); + if (!is_subset && !ns_capable(__task_cred(p)->user_ns, CAP_SYS_NICE)) + ret = -EPERM; rcu_read_unlock(); - if (!is_subset && !capable(CAP_SYS_NICE)) - return -EPERM; - return 0; + return ret; } /** -- cgit v1.2.3 From 71ac7f6255c560716c20da8ee2c964bbd96e941f Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Sun, 29 Sep 2013 08:39:21 -0700 Subject: apparmor: Use shash crypto API interface for profile hashes 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 Acked-by: Seth Arnold Signed-off-by: John Johansen Signed-off-by: James Morris --- security/apparmor/crypto.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) (limited to 'security') diff --git a/security/apparmor/crypto.c b/security/apparmor/crypto.c index d6222ba4e919..532471d0b3a0 100644 --- a/security/apparmor/crypto.c +++ b/security/apparmor/crypto.c @@ -15,14 +15,14 @@ * it should be. */ -#include +#include #include "include/apparmor.h" #include "include/crypto.h" static unsigned int apparmor_hash_size; -static struct crypto_hash *apparmor_tfm; +static struct crypto_shash *apparmor_tfm; unsigned int aa_hash_size(void) { @@ -32,35 +32,33 @@ unsigned int aa_hash_size(void) int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start, size_t len) { - struct scatterlist sg[2]; - struct hash_desc desc = { - .tfm = apparmor_tfm, - .flags = 0 - }; + struct { + struct shash_desc shash; + char ctx[crypto_shash_descsize(apparmor_tfm)]; + } desc; int error = -ENOMEM; u32 le32_version = cpu_to_le32(version); if (!apparmor_tfm) return 0; - sg_init_table(sg, 2); - sg_set_buf(&sg[0], &le32_version, 4); - sg_set_buf(&sg[1], (u8 *) start, len); - profile->hash = kzalloc(apparmor_hash_size, GFP_KERNEL); if (!profile->hash) goto fail; - error = crypto_hash_init(&desc); + desc.shash.tfm = apparmor_tfm; + desc.shash.flags = 0; + + error = crypto_shash_init(&desc.shash); if (error) goto fail; - error = crypto_hash_update(&desc, &sg[0], 4); + error = crypto_shash_update(&desc.shash, (u8 *) &le32_version, 4); if (error) goto fail; - error = crypto_hash_update(&desc, &sg[1], len); + error = crypto_shash_update(&desc.shash, (u8 *) start, len); if (error) goto fail; - error = crypto_hash_final(&desc, profile->hash); + error = crypto_shash_final(&desc.shash, profile->hash); if (error) goto fail; @@ -75,19 +73,19 @@ fail: static int __init init_profile_hash(void) { - struct crypto_hash *tfm; + struct crypto_shash *tfm; if (!apparmor_initialized) return 0; - tfm = crypto_alloc_hash("sha1", 0, CRYPTO_ALG_ASYNC); + tfm = crypto_alloc_shash("sha1", 0, CRYPTO_ALG_ASYNC); if (IS_ERR(tfm)) { int error = PTR_ERR(tfm); AA_ERROR("failed to setup profile sha1 hashing: %d\n", error); return error; } apparmor_tfm = tfm; - apparmor_hash_size = crypto_hash_digestsize(apparmor_tfm); + apparmor_hash_size = crypto_shash_digestsize(apparmor_tfm); aa_info_message("AppArmor sha1 policy hashing enabled"); -- cgit v1.2.3 From 4cd4fc77032dca46fe7475d81461e29145db247a Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sun, 29 Sep 2013 08:39:22 -0700 Subject: apparmor: fix suspicious RCU usage warning in policy.c/policy.h The recent 3.12 pull request for apparmor was missing a couple rcu _protected access modifiers. Resulting in the follow suspicious RCU usage [ 29.804534] [ INFO: suspicious RCU usage. ] [ 29.804539] 3.11.0+ #5 Not tainted [ 29.804541] ------------------------------- [ 29.804545] security/apparmor/include/policy.h:363 suspicious rcu_dereference_check() usage! [ 29.804548] [ 29.804548] other info that might help us debug this: [ 29.804548] [ 29.804553] [ 29.804553] rcu_scheduler_active = 1, debug_locks = 1 [ 29.804558] 2 locks held by apparmor_parser/1268: [ 29.804560] #0: (sb_writers#9){.+.+.+}, at: [] file_start_write+0x27/0x29 [ 29.804576] #1: (&ns->lock){+.+.+.}, at: [] aa_replace_profiles+0x166/0x57c [ 29.804589] [ 29.804589] stack backtrace: [ 29.804595] CPU: 0 PID: 1268 Comm: apparmor_parser Not tainted 3.11.0+ #5 [ 29.804599] Hardware name: ASUSTeK Computer Inc. UL50VT /UL50VT , BIOS 217 03/01/2010 [ 29.804602] 0000000000000000 ffff8800b95a1d90 ffffffff8144eb9b ffff8800b94db540 [ 29.804611] ffff8800b95a1dc0 ffffffff81087439 ffff880138cc3a18 ffff880138cc3a18 [ 29.804619] ffff8800b9464a90 ffff880138cc3a38 ffff8800b95a1df0 ffffffff811f5084 [ 29.804628] Call Trace: [ 29.804636] [] dump_stack+0x4e/0x82 [ 29.804642] [] lockdep_rcu_suspicious+0xfc/0x105 [ 29.804649] [] __aa_update_replacedby+0x53/0x7f [ 29.804655] [] __replace_profile+0x11f/0x1ed [ 29.804661] [] aa_replace_profiles+0x410/0x57c [ 29.804668] [] profile_replace+0x35/0x4c [ 29.804674] [] vfs_write+0xad/0x113 [ 29.804680] [] SyS_write+0x44/0x7a [ 29.804687] [] system_call_fastpath+0x16/0x1b [ 29.804691] [ 29.804694] =============================== [ 29.804697] [ INFO: suspicious RCU usage. ] [ 29.804700] 3.11.0+ #5 Not tainted [ 29.804703] ------------------------------- [ 29.804706] security/apparmor/policy.c:566 suspicious rcu_dereference_check() usage! [ 29.804709] [ 29.804709] other info that might help us debug this: [ 29.804709] [ 29.804714] [ 29.804714] rcu_scheduler_active = 1, debug_locks = 1 [ 29.804718] 2 locks held by apparmor_parser/1268: [ 29.804721] #0: (sb_writers#9){.+.+.+}, at: [] file_start_write+0x27/0x29 [ 29.804733] #1: (&ns->lock){+.+.+.}, at: [] aa_replace_profiles+0x166/0x57c [ 29.804744] [ 29.804744] stack backtrace: [ 29.804750] CPU: 0 PID: 1268 Comm: apparmor_parser Not tainted 3.11.0+ #5 [ 29.804753] Hardware name: ASUSTeK Computer Inc. UL50VT /UL50VT , BIOS 217 03/01/2010 [ 29.804756] 0000000000000000 ffff8800b95a1d80 ffffffff8144eb9b ffff8800b94db540 [ 29.804764] ffff8800b95a1db0 ffffffff81087439 ffff8800b95b02b0 0000000000000000 [ 29.804772] ffff8800b9efba08 ffff880138cc3a38 ffff8800b95a1dd0 ffffffff811f4f94 [ 29.804779] Call Trace: [ 29.804786] [] dump_stack+0x4e/0x82 [ 29.804791] [] lockdep_rcu_suspicious+0xfc/0x105 [ 29.804798] [] aa_free_replacedby_kref+0x4d/0x62 [ 29.804804] [] ? aa_put_namespace+0x17/0x17 [ 29.804810] [] kref_put+0x36/0x40 [ 29.804816] [] __replace_profile+0x13a/0x1ed [ 29.804822] [] aa_replace_profiles+0x410/0x57c [ 29.804829] [] profile_replace+0x35/0x4c [ 29.804835] [] vfs_write+0xad/0x113 [ 29.804840] [] SyS_write+0x44/0x7a [ 29.804847] [] system_call_fastpath+0x16/0x1b Reported-by: miles.lane@gmail.com CC: paulmck@linux.vnet.ibm.com Signed-off-by: John Johansen Signed-off-by: James Morris --- security/apparmor/include/policy.h | 4 +++- security/apparmor/policy.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index f2d4b6348cbc..c28b0f20ab53 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -360,7 +360,9 @@ static inline void aa_put_replacedby(struct aa_replacedby *p) static inline void __aa_update_replacedby(struct aa_profile *orig, struct aa_profile *new) { - struct aa_profile *tmp = rcu_dereference(orig->replacedby->profile); + struct aa_profile *tmp; + tmp = rcu_dereference_protected(orig->replacedby->profile, + mutex_is_locked(&orig->ns->lock)); rcu_assign_pointer(orig->replacedby->profile, aa_get_profile(new)); orig->flags |= PFLAG_INVALID; aa_put_profile(tmp); diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 6172509fa2b7..345bec07a27d 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -563,7 +563,8 @@ void __init aa_free_root_ns(void) static void free_replacedby(struct aa_replacedby *r) { if (r) { - aa_put_profile(rcu_dereference(r->profile)); + /* r->profile will not be updated any more as r is dead */ + aa_put_profile(rcu_dereference_protected(r->profile, true)); kzfree(r); } } -- cgit v1.2.3 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 a5091ec06aa6..967823212d7d 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.2.3 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 dad36a6ab45f..e720f72fcb87 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 92d0ab561db8..e30657b59cb3 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.2.3 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 e720f72fcb87..fc3e6628a864 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 967823212d7d..5b5231068516 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 e30657b59cb3..f53ee3c58d0f 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.2.3 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 345bec07a27d..705c2879d3a9 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.2.3 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 95c2b2689a03..7db9954f1af2 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.2.3