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;