From a4987857d2c958b93b2faafe0811eea1a63ff59a Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:10:34 -0800 Subject: apparmor: remove sid from profiles The sid is not going to be a direct property of a profile anymore, instead it will be directly related to the label, and the profile will pickup a label back reference. For null-profiles replace the use of sid with a per namespace unique id. Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/policy.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) (limited to 'security/apparmor/policy.c') diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 813200384d97..13fc9efddd5d 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -87,7 +87,6 @@ #include "include/policy.h" #include "include/policy_unpack.h" #include "include/resource.h" -#include "include/sid.h" /* root profile namespace */ @@ -292,7 +291,6 @@ static struct aa_namespace *alloc_namespace(const char *prefix, if (!ns->unconfined) goto fail_unconfined; - ns->unconfined->sid = aa_alloc_sid(); ns->unconfined->flags = PFLAG_UNCONFINED | PFLAG_IX_ON_NAME_ERROR | PFLAG_IMMUTABLE; @@ -303,6 +301,8 @@ static struct aa_namespace *alloc_namespace(const char *prefix, */ ns->unconfined->ns = aa_get_namespace(ns); + atomic_set(&ns->uniq_null, 0); + return ns; fail_unconfined: @@ -497,7 +497,6 @@ static void __replace_profile(struct aa_profile *old, struct aa_profile *new) /* released when @new is freed */ new->parent = aa_get_profile(old->parent); new->ns = aa_get_namespace(old->ns); - new->sid = old->sid; __list_add_profile(&policy->profiles, new); /* inherit children */ list_for_each_entry_safe(child, tmp, &old->base.profiles, base.list) { @@ -665,7 +664,7 @@ struct aa_profile *aa_alloc_profile(const char *hname) * @hat: true if the null- learning profile is a hat * * Create a null- complain mode profile used in learning mode. The name of - * the profile is unique and follows the format of parent//null-sid. + * the profile is unique and follows the format of parent//null-. * * null profiles are added to the profile list but the list does not * hold a count on them so that they are automatically released when @@ -677,20 +676,19 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) { struct aa_profile *profile = NULL; char *name; - u32 sid = aa_alloc_sid(); + int uniq = atomic_inc_return(&parent->ns->uniq_null); /* freed below */ name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, GFP_KERNEL); if (!name) goto fail; - sprintf(name, "%s//null-%x", parent->base.hname, sid); + sprintf(name, "%s//null-%x", parent->base.hname, uniq); profile = aa_alloc_profile(name); kfree(name); if (!profile) goto fail; - profile->sid = sid; profile->mode = APPARMOR_COMPLAIN; profile->flags = PFLAG_NULL; if (hat) @@ -708,7 +706,6 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) return profile; fail: - aa_free_sid(sid); return NULL; } @@ -749,7 +746,6 @@ static void free_profile(struct aa_profile *profile) aa_free_cap_rules(&profile->caps); aa_free_rlimit_rules(&profile->rlimits); - aa_free_sid(profile->sid); aa_put_dfa(profile->xmatch); aa_put_dfa(profile->policy.dfa); @@ -972,7 +968,6 @@ static void __add_new_profile(struct aa_namespace *ns, struct aa_policy *policy, profile->parent = aa_get_profile((struct aa_profile *) policy); __list_add_profile(&policy->profiles, profile); /* released on free_profile */ - profile->sid = aa_alloc_sid(); profile->ns = aa_get_namespace(ns); } @@ -1110,14 +1105,8 @@ audit: if (!error) { if (rename_profile) __replace_profile(rename_profile, new_profile); - if (old_profile) { - /* when there are both rename and old profiles - * inherit old profiles sid - */ - if (rename_profile) - aa_free_sid(new_profile->sid); + if (old_profile) __replace_profile(old_profile, new_profile); - } if (!(old_profile || rename_profile)) __add_new_profile(ns, policy, new_profile); } -- cgit v1.2.3 From 4da05cc08da3f2058cecbe42ed9f4803d669730a Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 18 Feb 2013 16:11:34 -0800 Subject: apparmor: move the free_profile fn ahead of aa_alloc_profile Move the free_profile fn ahead of aa_alloc_profile so it can be used in aa_alloc_profile without a forward declaration. Signed-off-by: John Johansen Acked-by: Kees Cook --- security/apparmor/policy.c | 150 ++++++++++++++++++++++----------------------- 1 file changed, 75 insertions(+), 75 deletions(-) (limited to 'security/apparmor/policy.c') diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 13fc9efddd5d..f4ee72b44de4 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -634,81 +634,6 @@ void __init aa_free_root_ns(void) aa_put_namespace(ns); } -/** - * aa_alloc_profile - allocate, initialize and return a new profile - * @hname: name of the profile (NOT NULL) - * - * Returns: refcount profile or NULL on failure - */ -struct aa_profile *aa_alloc_profile(const char *hname) -{ - struct aa_profile *profile; - - /* freed by free_profile - usually through aa_put_profile */ - profile = kzalloc(sizeof(*profile), GFP_KERNEL); - if (!profile) - return NULL; - - if (!policy_init(&profile->base, NULL, hname)) { - kzfree(profile); - return NULL; - } - - /* refcount released by caller */ - return profile; -} - -/** - * aa_new_null_profile - create a new null-X learning profile - * @parent: profile that caused this profile to be created (NOT NULL) - * @hat: true if the null- learning profile is a hat - * - * Create a null- complain mode profile used in learning mode. The name of - * the profile is unique and follows the format of parent//null-. - * - * null profiles are added to the profile list but the list does not - * hold a count on them so that they are automatically released when - * not in use. - * - * Returns: new refcounted profile else NULL on failure - */ -struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) -{ - struct aa_profile *profile = NULL; - char *name; - int uniq = atomic_inc_return(&parent->ns->uniq_null); - - /* freed below */ - name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, GFP_KERNEL); - if (!name) - goto fail; - sprintf(name, "%s//null-%x", parent->base.hname, uniq); - - profile = aa_alloc_profile(name); - kfree(name); - if (!profile) - goto fail; - - profile->mode = APPARMOR_COMPLAIN; - profile->flags = PFLAG_NULL; - if (hat) - profile->flags |= PFLAG_HAT; - - /* released on free_profile */ - profile->parent = aa_get_profile(parent); - profile->ns = aa_get_namespace(parent->ns); - - write_lock(&profile->ns->lock); - __list_add_profile(&parent->base.profiles, profile); - write_unlock(&profile->ns->lock); - - /* refcount released by caller */ - return profile; - -fail: - return NULL; -} - /** * free_profile - free a profile * @profile: the profile to free (MAYBE NULL) @@ -786,6 +711,81 @@ void aa_free_profile_kref(struct kref *kref) free_profile(p); } +/** + * aa_alloc_profile - allocate, initialize and return a new profile + * @hname: name of the profile (NOT NULL) + * + * Returns: refcount profile or NULL on failure + */ +struct aa_profile *aa_alloc_profile(const char *hname) +{ + struct aa_profile *profile; + + /* freed by free_profile - usually through aa_put_profile */ + profile = kzalloc(sizeof(*profile), GFP_KERNEL); + if (!profile) + return NULL; + + if (!policy_init(&profile->base, NULL, hname)) { + kzfree(profile); + return NULL; + } + + /* refcount released by caller */ + return profile; +} + +/** + * aa_new_null_profile - create a new null-X learning profile + * @parent: profile that caused this profile to be created (NOT NULL) + * @hat: true if the null- learning profile is a hat + * + * Create a null- complain mode profile used in learning mode. The name of + * the profile is unique and follows the format of parent//null-. + * + * null profiles are added to the profile list but the list does not + * hold a count on them so that they are automatically released when + * not in use. + * + * Returns: new refcounted profile else NULL on failure + */ +struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) +{ + struct aa_profile *profile = NULL; + char *name; + int uniq = atomic_inc_return(&parent->ns->uniq_null); + + /* freed below */ + name = kmalloc(strlen(parent->base.hname) + 2 + 7 + 8, GFP_KERNEL); + if (!name) + goto fail; + sprintf(name, "%s//null-%x", parent->base.hname, uniq); + + profile = aa_alloc_profile(name); + kfree(name); + if (!profile) + goto fail; + + profile->mode = APPARMOR_COMPLAIN; + profile->flags = PFLAG_NULL; + if (hat) + profile->flags |= PFLAG_HAT; + + /* released on free_profile */ + profile->parent = aa_get_profile(parent); + profile->ns = aa_get_namespace(parent->ns); + + write_lock(&profile->ns->lock); + __list_add_profile(&parent->base.profiles, profile); + write_unlock(&profile->ns->lock); + + /* refcount released by caller */ + return profile; + +fail: + return NULL; +} + /* TODO: profile accounting - setup in remove */ /** -- cgit v1.2.3 From 41d1b3e868c263e8b43dd5903a70633e05ae58a6 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 21 Feb 2013 01:14:17 -0800 Subject: apparmor: Fix smatch warning in aa_remove_profiles smatch reports error: potential NULL dereference 'ns'. this can not actually occur because it relies on aa_split_fqname setting both ns_name and name as null but ns_name will actually always have a value in this case. so remove the unnecessary if (ns_name) conditional that is resulting in the false positive further down. Signed-off-by: John Johansen --- security/apparmor/policy.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'security/apparmor/policy.c') diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index f4ee72b44de4..0f345c4dee5f 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -1156,14 +1156,12 @@ ssize_t aa_remove_profiles(char *fqname, size_t size) if (fqname[0] == ':') { char *ns_name; name = aa_split_fqname(fqname, &ns_name); - if (ns_name) { - /* released below */ - ns = aa_find_namespace(root, ns_name); - if (!ns) { - info = "namespace does not exist"; - error = -ENOENT; - goto fail; - } + /* released below */ + ns = aa_find_namespace(root, ns_name); + if (!ns) { + info = "namespace does not exist"; + error = -ENOENT; + goto fail; } } else /* released below */ -- cgit v1.2.3