moving replacedby to use labels will allow for faster label lookup when a label is replaced and also remove the need to create replacement labels to get a none stale version during task confinement lookup.
this is a first step just handling single profile labels, a second patch to actually handle compound (multi-profile) labels will be needed Signed-off-by: John Johansen <[email protected]> --- security/apparmor/apparmorfs.c | 17 ++++--- security/apparmor/context.c | 4 +- security/apparmor/domain.c | 6 +-- security/apparmor/include/context.h | 2 +- security/apparmor/include/label.h | 77 +++++++++++++++++++++++++++++++- security/apparmor/include/policy.h | 51 --------------------- security/apparmor/label.c | 30 +++++++++++++ security/apparmor/lsm.c | 6 +-- security/apparmor/policy.c | 89 +++++++++++++++++++------------------ 9 files changed, 170 insertions(+), 112 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index e4d6853..3b861e4 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -248,9 +248,10 @@ static int aa_fs_seq_profile_release(struct inode *inode, struct file *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); + struct aa_label *label = aa_get_label_rcu(&r->label); + struct aa_profile *profile = labels_profile(label); seq_printf(seq, "%s\n", profile->base.name); - aa_put_profile(profile); + aa_put_label(label); return 0; } @@ -271,9 +272,10 @@ static const struct file_operations aa_fs_profname_fops = { 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); + struct aa_label *label = aa_get_label_rcu(&r->label); + struct aa_profile *profile = labels_profile(label); seq_printf(seq, "%s\n", aa_profile_mode_names[profile->mode]); - aa_put_profile(profile); + aa_put_label(label); return 0; } @@ -294,14 +296,15 @@ static const struct file_operations aa_fs_profmode_fops = { 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); + struct aa_label *label = aa_get_label_rcu(&r->label); + struct aa_profile *profile = labels_profile(label); if (profile->attach) seq_printf(seq, "%s\n", profile->attach); else if (profile->xmatch) seq_printf(seq, "<unknown>\n"); else seq_printf(seq, "%s\n", profile->base.name); - aa_put_profile(profile); + aa_put_label(label); return 0; } @@ -358,7 +361,7 @@ 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 aa_replacedby *r = aa_get_replacedby(profile->label.replacedby); struct dentry *dent; dent = securityfs_create_file(name, S_IFREG | 0444, dir, r, fops); diff --git a/security/apparmor/context.c b/security/apparmor/context.c index 3064c6c..355c4ed 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/context.c @@ -175,7 +175,7 @@ int aa_set_current_hat(struct aa_profile *profile, u64 token) abort_creds(new); return -EACCES; } - cxt->profile = aa_get_newest_profile(profile); + cxt->profile = labels_profile(aa_get_newest_label(&profile->label)); /* clear exec on switching context */ aa_put_profile(cxt->onexec); cxt->onexec = NULL; @@ -212,7 +212,7 @@ int aa_restore_previous_profile(u64 token) } aa_put_profile(cxt->profile); - cxt->profile = aa_get_newest_profile(cxt->previous); + cxt->profile = labels_profile(aa_get_newest_label(&cxt->previous->label)); BUG_ON(!cxt->profile); /* 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 ccf19eb..66454ad 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_newest_profile(cxt->profile); + profile = labels_profile(aa_get_newest_label(&cxt->profile->label)); /* * 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_newest_profile(cxt->onexec); + new_profile = labels_profile(aa_get_newest_label(&cxt->onexec->label)); goto apply; } @@ -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_newest_profile(ns->unconfined); + new_profile = labels_profile(aa_get_newest_label(&ns->unconfined->label)); info = "ux fallback"; } else { error = -ENOENT; diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h index 6bf6579..10c4a30 100644 --- a/security/apparmor/include/context.h +++ b/security/apparmor/include/context.h @@ -153,7 +153,7 @@ static inline struct aa_profile *aa_current_profile(void) BUG_ON(!cxt || !cxt->profile); if (PROFILE_INVALID(cxt->profile)) { - profile = aa_get_newest_profile(cxt->profile); + profile = labels_profile(aa_get_newest_label(&cxt->profile->label)); aa_replace_current_profile(profile); aa_put_profile(profile); cxt = current_cxt(); diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h index a471d0b..2ff1eb0 100644 --- a/security/apparmor/include/label.h +++ b/security/apparmor/include/label.h @@ -120,11 +120,18 @@ enum label_flags { FLAG_MEDIATE_DELETED = 0x10000, /* mediate instead delegate deleted */ }; +struct aa_label; +struct aa_replacedby { + struct kref count; + struct aa_label __rcu *label; +}; + /* struct aa_label - lazy labeling struct * @count: ref count of active users * @node: rbtree position * @rcu: rcu callback struct - * @name: text representation of the label (MAYBE_NULL) + * @replacedby: is set to the label that replaced this label + * @hname: text representation of the label (MAYBE_NULL) * @flags: invalid and other flags - values may change under label set lock * @sid: sid that references this label * @size: number of entries in @ent[] @@ -134,6 +141,7 @@ struct aa_label { struct kref count; struct rb_node node; struct rcu_head rcu; + struct aa_replacedby *replacedby; __counted char *hname; long flags; u32 sid; @@ -206,10 +214,77 @@ static inline struct aa_label *aa_get_label_not0(struct aa_label *l) return NULL; } +/** + * aa_get_label_rcu - increment refcount on a label that can be replaced + * @l: pointer to label that can be replaced (NOT NULL) + * + * Returns: pointer to a refcounted label. + * else NULL if no label + */ +static inline struct aa_label *aa_get_label_rcu(struct aa_label __rcu **l) +{ + struct aa_label *c; + + rcu_read_lock(); + do { + c = rcu_dereference(*l); + } while (c && !kref_get_not0(&c->count)); + rcu_read_unlock(); + + return c; +} + +/** + * aa_get_newest_label - find the newest version of @l + * @l: the label to check for newer versions of + * + * Returns: refcounted newest version of @l taking into account + * replacement, renames and removals + * return @l. + */ +static inline struct aa_label *aa_get_newest_label(struct aa_label *l) +{ + if (!l) + return NULL; + + if (label_invalid(l)) + return aa_get_label_rcu(&l->replacedby->label); + + return aa_get_label(l); +} + static inline void aa_put_label(struct aa_label *l) { if (l) kref_put(&l->count, aa_label_kref); } + +struct aa_replacedby *aa_alloc_replacedby(struct aa_label *l); +void aa_free_replacedby_kref(struct kref *kref); + +static inline struct aa_replacedby *aa_get_replacedby(struct aa_replacedby *r) +{ + if (r) + kref_get(&(r->count)); + + return r; +} + +static inline void aa_put_replacedby(struct aa_replacedby *r) +{ + if (r) + kref_put(&r->count, aa_free_replacedby_kref); +} + +/* requires profile list write lock held */ +static inline void __aa_update_replacedby(struct aa_label *orig, + struct aa_label *new) +{ + struct aa_label *tmp = rcu_dereference(orig->replacedby->label); + rcu_assign_pointer(orig->replacedby->label, aa_get_label(new)); + orig->flags |= FLAG_INVALID; + aa_put_label(tmp); +} + #endif /* __AA_LABEL_H */ diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 0a29372..932395d 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -139,16 +139,10 @@ 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 * @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 @@ -187,7 +181,6 @@ struct aa_profile { struct aa_profile __rcu *parent; struct aa_namespace *ns; - struct aa_replacedby *replacedby; const char *rename; const char *attach; @@ -223,7 +216,6 @@ struct aa_namespace *aa_find_namespace(struct aa_namespace *root, const char *name); -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); struct aa_profile *aa_setup_default_profile(void); @@ -296,25 +288,6 @@ static inline struct aa_profile *aa_get_profile_rcu(struct aa_profile __rcu **p) } /** - * 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) */ @@ -324,30 +297,6 @@ static inline void aa_put_profile(struct aa_profile *p) kref_put(&p->label.count, aa_label_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->label.flags |= FLAG_INVALID; - aa_put_profile(tmp); -} - /** * aa_get_namespace - increment references count on @ns * @ns: namespace to increment reference count of (MAYBE NULL) diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 9a74c50..0c67a80 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -55,6 +55,35 @@ #define AA_WARN(X) WARN((X), "APPARMOR WARN %s: %s\n", __FUNCTION__, #X) + +static void free_replacedby(struct aa_replacedby *r) +{ + if (r) { + aa_put_label(rcu_dereference(r->label)); + kzfree(r); + } +} + +void aa_free_replacedby_kref(struct kref *kref) +{ + struct aa_replacedby *r = container_of(kref, struct aa_replacedby, + count); + free_replacedby(r); +} + +struct aa_replacedby *aa_alloc_replacedby(struct aa_label *l) +{ + struct aa_replacedby *r; + + r = kzalloc(sizeof(struct aa_replacedby), GFP_KERNEL); + if (r) { + kref_init(&r->count); + rcu_assign_pointer(r->label, aa_get_label(l)); + } + return r; +} + + /* helper fn for label_for_each_confined * - skip delegation? How exactly? * - skip profiles being cached on label with explicit rule access @@ -103,6 +132,7 @@ void aa_label_destroy(struct aa_label *label) } aa_free_sid(label->sid); + aa_put_replacedby(label->replacedby); } void aa_label_free(struct aa_label *label) diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 8e51e2e..31fb46d 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -511,11 +511,11 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, struct aa_profile *profile = NULL; if (strcmp(name, "current") == 0) - profile = aa_get_newest_profile(cxt->profile); + profile = labels_profile(aa_get_newest_label(&cxt->profile->label)); else if (strcmp(name, "prev") == 0 && cxt->previous) - profile = aa_get_newest_profile(cxt->previous); + profile = labels_profile(aa_get_newest_label(&cxt->previous->label)); else if (strcmp(name, "exec") == 0 && cxt->onexec) - profile = aa_get_newest_profile(cxt->onexec); + profile = labels_profile(aa_get_newest_label(&cxt->onexec->label)); else error = -EINVAL; diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 9d946ce..8cf2eed 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -298,6 +298,9 @@ static struct aa_namespace *alloc_namespace(const char *prefix, ns->unconfined = aa_alloc_profile("unconfined"); if (!ns->unconfined) goto fail_unconfined; + ns->unconfined->label.replacedby = aa_alloc_replacedby(NULL); + if (!ns->unconfined->label.replacedby) + goto fail_replacedby; ns->unconfined->label.flags |= FLAG_IX_ON_NAME_ERROR | FLAG_IMMUTABLE | FLAG_NS_COUNT | FLAG_UNCONFINED; @@ -312,6 +315,9 @@ static struct aa_namespace *alloc_namespace(const char *prefix, return ns; +fail_replacedby: + aa_free_profile(ns->unconfined); + fail_unconfined: kzfree(ns->base.hname); fail_ns: @@ -474,7 +480,8 @@ static void __remove_profile(struct aa_profile *profile) /* release any children lists first */ __profile_list_release(&profile->base.profiles); /* released by free_profile */ - __aa_update_replacedby(profile, profile->ns->unconfined); + __aa_update_replacedby(&profile->label, + &profile->ns->unconfined->label); __aa_fs_profile_rmdir(profile); __list_remove_profile(profile); } @@ -511,7 +518,8 @@ static void destroy_namespace(struct aa_namespace *ns) __ns_list_release(&ns->sub_ns); if (ns->parent) - __aa_update_replacedby(ns->unconfined, ns->parent->unconfined); + __aa_update_replacedby(&ns->unconfined->label, + &ns->parent->unconfined->label); __aa_fs_namespace_rmdir(ns); mutex_unlock(&ns->lock); } @@ -573,22 +581,6 @@ void __init aa_free_root_ns(void) } -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); -} - /** * aa_free_profile - free a profile * @profile: the profile to free (MAYBE NULL) @@ -627,7 +619,6 @@ void aa_free_profile(struct aa_profile *profile) kzfree(profile->dirname); aa_put_dfa(profile->xmatch); aa_put_dfa(profile->policy.dfa); - aa_put_replacedby(profile->replacedby); kzfree(profile); } @@ -647,11 +638,6 @@ struct aa_profile *aa_alloc_profile(const char *hname) if (!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; if (!aa_label_init(&profile->label, 1)) @@ -664,7 +650,6 @@ struct aa_profile *aa_alloc_profile(const char *hname) return profile; fail: - kzfree(profile->replacedby); kzfree(profile); return NULL; @@ -701,6 +686,10 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) if (!profile) goto fail; + profile->label.replacedby = aa_alloc_replacedby(NULL); + if (!profile->label.replacedby) + goto fail; + profile->mode = APPARMOR_COMPLAIN; profile->label.flags |= FLAG_NULL; if (hat) @@ -718,6 +707,7 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) return profile; fail: + aa_free_profile(profile); return NULL; } @@ -737,8 +727,11 @@ struct aa_profile *aa_setup_default_profile(void) profile->ns = aa_get_namespace(root_ns); /* replacedby being set needed by fs interface */ - rcu_assign_pointer(profile->replacedby->profile, - aa_get_profile(profile)); + profile->label.replacedby = aa_alloc_replacedby(&profile->label); + if (!profile->label.replacedby) { + aa_free_profile(profile); + return NULL; + } __list_add_profile(&root_ns->base.profiles, profile); return profile; @@ -893,7 +886,7 @@ struct aa_profile *aa_lookupn_profile(struct aa_namespace *ns, /* the unconfined profile is not in the regular profile list */ if (!profile && strncmp(hname, "unconfined", n) == 0) - profile = aa_get_newest_profile(ns->unconfined); + profile = labels_profile(aa_get_newest_label(&ns->unconfined->label)); /* refcount released by caller */ return profile; @@ -1047,14 +1040,13 @@ 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)); } - __aa_update_replacedby(old, new); - if (old_replacedby) { - aa_put_replacedby(new->replacedby); - new->replacedby = aa_get_replacedby(old->replacedby); - } else + __aa_update_replacedby(&old->label, &new->label); + if (old_replacedby) + new->label.replacedby = aa_get_replacedby(old->label.replacedby); + else /* aafs interface uses replacedby */ - rcu_assign_pointer(new->replacedby->profile, - aa_get_profile(new)); + rcu_assign_pointer(new->label.replacedby->label, + aa_get_label(&new->label)); __aa_fs_profile_migrate_dents(old, new); if (list_empty_rcu(&new->base.list)) { @@ -1108,7 +1100,7 @@ static struct aa_profile *update_to_newest_parent(struct aa_profile *new) struct aa_profile *parent, *newest; parent = rcu_dereference_protected(new->parent, mutex_is_locked(&new->ns->lock)); - newest = aa_get_newest_profile(parent); + newest = labels_profile(aa_get_newest_label(&parent->label)); /* parent replaced in this atomic set? */ if (newest != parent) { @@ -1201,14 +1193,23 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) /* create new fs entries for introspection if needed */ list_for_each_entry(ent, &lh, list) { + struct aa_replacedby *r; if (ent->old) { if (ent->rename) { - // ??? + // ??? + alloc replacedby } } else if (ent->rename) { - // ???? + // ???? + alloc replacedby } else { struct dentry *parent; + r = aa_alloc_replacedby(NULL); + if (!r) { + info = "failed to create "; + error = -ENOMEM; + goto fail_lock; + } + ent->new->label.replacedby = r; + if (rcu_access_pointer(ent->new->parent)) { struct aa_profile *p; p = rcu_dereference_protected(ent->new->parent, @@ -1240,14 +1241,14 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) __aa_labelset_invalidate_all(ns, ent->old); if (ent->rename) { /* aafs interface uses replacedby */ - rcu_assign_pointer(ent->new->replacedby->profile, - aa_get_profile(ent->new)); + rcu_assign_pointer(ent->new->label.replacedby->label, + aa_get_label(&ent->new->label)); __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)); + rcu_assign_pointer(ent->new->label.replacedby->label, + aa_get_label(&ent->new->label)); __replace_profile(ent->rename, ent->new, 0); } else { struct aa_label *l; @@ -1260,8 +1261,8 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace) } else lh = &ns->base.profiles; /* aafs interface uses replacedby */ - rcu_assign_pointer(ent->new->replacedby->profile, - aa_get_profile(ent->new)); + rcu_assign_pointer(ent->new->label.replacedby->label, + aa_get_label(&ent->new->label)); __list_add_profile(lh, ent->new); l = aa_label_insert(&ns->labels, &ent->new->label); aa_put_label(l); -- 1.8.1.2 -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
