-- cgit v1.2.3 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 --- fs/ocfs2/xattr.h | 2 +- include/linux/security.h | 8 ++++---- include/linux/xattr.h | 2 +- include/uapi/linux/reiserfs_xattr.h | 2 +- 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 +++------ 9 files changed, 21 insertions(+), 31 deletions(-) diff --git a/fs/ocfs2/xattr.h b/fs/ocfs2/xattr.h index e5c7f15465b4..19f134e896a9 100644 --- a/fs/ocfs2/xattr.h +++ b/fs/ocfs2/xattr.h @@ -32,7 +32,7 @@ enum ocfs2_xattr_type { struct ocfs2_security_xattr_info { int enable; - char *name; + const char *name; void *value; size_t value_len; }; diff --git a/include/linux/security.h b/include/linux/security.h index 7ce53ae1266b..9d37e2b9d3ec 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1492,7 +1492,7 @@ struct security_operations { int (*inode_alloc_security) (struct inode *inode); void (*inode_free_security) (struct inode *inode); int (*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); int (*inode_create) (struct inode *dir, struct dentry *dentry, umode_t mode); @@ -1770,7 +1770,7 @@ int security_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, initxattrs initxattrs, void *fs_data); 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); int security_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode); int security_inode_link(struct dentry *old_dentry, struct inode *dir, @@ -2094,8 +2094,8 @@ static inline int security_inode_init_security(struct inode *inode, static inline int security_old_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, - char **name, void **value, - size_t *len) + const char **name, + void **value, size_t *len) { return -EOPNOTSUPP; } diff --git a/include/linux/xattr.h b/include/linux/xattr.h index fdbafc6841cf..91b0a68d38dc 100644 --- a/include/linux/xattr.h +++ b/include/linux/xattr.h @@ -31,7 +31,7 @@ struct xattr_handler { }; struct xattr { - char *name; + const char *name; void *value; size_t value_len; }; diff --git a/include/uapi/linux/reiserfs_xattr.h b/include/uapi/linux/reiserfs_xattr.h index d8ce17c2459a..38fdd648be21 100644 --- a/include/uapi/linux/reiserfs_xattr.h +++ b/include/uapi/linux/reiserfs_xattr.h @@ -16,7 +16,7 @@ struct reiserfs_xattr_header { }; struct reiserfs_security_handle { - char *name; + const char *name; void *value; size_t length; }; 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 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(-) 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(-) 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(-) 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 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(-) 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(-) 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(-) 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(+) 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(-) 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(-) 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(-) 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(-) 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(-) 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(-) 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(-) 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(-) 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(+) 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(+) 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(-) 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 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