From c092921219d227b13cb80dbecd3545ee66ab89b3 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 31 Jul 2017 17:36:45 -0700 Subject: apparmor: add support for mapping secids and using secctxes Use a radix tree to provide a map between the secid and the label, and along with it a basic ability to provide secctx conversion. Shared/cached secctx will be added later. Signed-off-by: John Johansen --- security/apparmor/include/label.h | 2 +- security/apparmor/include/secid.h | 15 ++- security/apparmor/label.c | 6 +- security/apparmor/lsm.c | 5 + security/apparmor/policy.c | 2 +- security/apparmor/secid.c | 219 +++++++++++++++++++++++++++++++++++--- 6 files changed, 224 insertions(+), 25 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h index d871e7ff0952..7ce5fe73ae7f 100644 --- a/security/apparmor/include/label.h +++ b/security/apparmor/include/label.h @@ -281,7 +281,7 @@ void __aa_labelset_update_subtree(struct aa_ns *ns); void aa_label_free(struct aa_label *label); void aa_label_kref(struct kref *kref); -bool aa_label_init(struct aa_label *label, int size); +bool aa_label_init(struct aa_label *label, int size, gfp_t gfp); struct aa_label *aa_label_alloc(int size, struct aa_proxy *proxy, gfp_t gfp); bool aa_label_is_subset(struct aa_label *set, struct aa_label *sub); diff --git a/security/apparmor/include/secid.h b/security/apparmor/include/secid.h index 95ed86a0f1e2..686de8e50a79 100644 --- a/security/apparmor/include/secid.h +++ b/security/apparmor/include/secid.h @@ -3,7 +3,7 @@ * * This file contains AppArmor security identifier (secid) definitions * - * Copyright 2009-2010 Canonical Ltd. + * Copyright 2009-2018 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 @@ -14,13 +14,22 @@ #ifndef __AA_SECID_H #define __AA_SECID_H +#include #include +struct aa_label; + /* secid value that will not be allocated */ #define AA_SECID_INVALID 0 -#define AA_SECID_ALLOC AA_SECID_INVALID -u32 aa_alloc_secid(void); +struct aa_label *aa_secid_to_label(u32 secid); +int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen); +int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid); +void apparmor_release_secctx(char *secdata, u32 seclen); + + +u32 aa_alloc_secid(struct aa_label *label, gfp_t gfp); void aa_free_secid(u32 secid); +void aa_secid_update(u32 secid, struct aa_label *label); #endif /* __AA_SECID_H */ diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 523250e34837..152352755869 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -402,12 +402,12 @@ static void label_free_or_put_new(struct aa_label *label, struct aa_label *new) aa_put_label(new); } -bool aa_label_init(struct aa_label *label, int size) +bool aa_label_init(struct aa_label *label, int size, gfp_t gfp) { AA_BUG(!label); AA_BUG(size < 1); - label->secid = aa_alloc_secid(); + label->secid = aa_alloc_secid(label, gfp); if (label->secid == AA_SECID_INVALID) return false; @@ -441,7 +441,7 @@ struct aa_label *aa_label_alloc(int size, struct aa_proxy *proxy, gfp_t gfp) if (!new) goto fail; - if (!aa_label_init(new, size)) + if (!aa_label_init(new, size, gfp)) goto fail; if (!proxy) { diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index ce2b89e9ad94..91284b5d56a3 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -39,6 +39,7 @@ #include "include/policy_ns.h" #include "include/procattr.h" #include "include/mount.h" +#include "include/secid.h" /* Flag indicating whether initialization completed */ int apparmor_initialized; @@ -1188,6 +1189,10 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(task_alloc, apparmor_task_alloc), LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit), LSM_HOOK_INIT(task_kill, apparmor_task_kill), + + LSM_HOOK_INIT(secid_to_secctx, apparmor_secid_to_secctx), + LSM_HOOK_INIT(secctx_to_secid, apparmor_secctx_to_secid), + LSM_HOOK_INIT(release_secctx, apparmor_release_secctx), }; /* diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index c07493ce2376..d68252b112dc 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -268,7 +268,7 @@ struct aa_profile *aa_alloc_profile(const char *hname, struct aa_proxy *proxy, if (!aa_policy_init(&profile->base, NULL, hname, gfp)) goto fail; - if (!aa_label_init(&profile->label, 1)) + if (!aa_label_init(&profile->label, 1, gfp)) goto fail; /* update being set needed by fs interface */ diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c index 3a3edbad0b21..502924853986 100644 --- a/security/apparmor/secid.c +++ b/security/apparmor/secid.c @@ -3,7 +3,7 @@ * * This file contains AppArmor security identifier (secid) manipulation fns * - * Copyright 2009-2010 Canonical Ltd. + * Copyright 2009-2017 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 @@ -11,37 +11,218 @@ * License. * * - * AppArmor allocates a unique secid for every profile loaded. If a profile - * is replaced it receives the secid of the profile it is replacing. - * - * The secid value of 0 is invalid. + * AppArmor allocates a unique secid for every label used. If a label + * is replaced it receives the secid of the label it is replacing. */ -#include #include #include +#include +#include +#include +#include "include/cred.h" +#include "include/lib.h" #include "include/secid.h" +#include "include/label.h" +#include "include/policy_ns.h" -/* global counter from which secids are allocated */ -static u32 global_secid; +/* + * secids - do not pin labels with a refcount. They rely on the label + * properly updating/freeing them + * + * A singly linked free list is used to track secids that have been + * freed and reuse them before allocating new ones + */ + +#define FREE_LIST_HEAD 1 + +static RADIX_TREE(aa_secids_map, GFP_ATOMIC); static DEFINE_SPINLOCK(secid_lock); +static u32 alloced_secid = FREE_LIST_HEAD; +static u32 free_list = FREE_LIST_HEAD; +static unsigned long free_count; + +/* + * TODO: allow policy to reserve a secid range? + * TODO: add secid pinning + * TODO: use secid_update in label replace + */ + +#define SECID_MAX U32_MAX + +/* TODO: mark free list as exceptional */ +static void *to_ptr(u32 secid) +{ + return (void *) + ((((unsigned long) secid) << RADIX_TREE_EXCEPTIONAL_SHIFT)); +} + +static u32 to_secid(void *ptr) +{ + return (u32) (((unsigned long) ptr) >> RADIX_TREE_EXCEPTIONAL_SHIFT); +} + + +/* TODO: tag free_list entries to mark them as different */ +static u32 __pop(struct aa_label *label) +{ + u32 secid = free_list; + void __rcu **slot; + void *entry; + + if (free_list == FREE_LIST_HEAD) + return AA_SECID_INVALID; + + slot = radix_tree_lookup_slot(&aa_secids_map, secid); + AA_BUG(!slot); + entry = radix_tree_deref_slot_protected(slot, &secid_lock); + free_list = to_secid(entry); + radix_tree_replace_slot(&aa_secids_map, slot, label); + free_count--; + + return secid; +} + +static void __push(u32 secid) +{ + void __rcu **slot; + + slot = radix_tree_lookup_slot(&aa_secids_map, secid); + AA_BUG(!slot); + radix_tree_replace_slot(&aa_secids_map, slot, to_ptr(free_list)); + free_list = secid; + free_count++; +} + +static struct aa_label * __secid_update(u32 secid, struct aa_label *label) +{ + struct aa_label *old; + void __rcu **slot; + + slot = radix_tree_lookup_slot(&aa_secids_map, secid); + AA_BUG(!slot); + old = radix_tree_deref_slot_protected(slot, &secid_lock); + radix_tree_replace_slot(&aa_secids_map, slot, label); + + return old; +} + +/** + * aa_secid_update - update a secid mapping to a new label + * @secid: secid to update + * @label: label the secid will now map to + */ +void aa_secid_update(u32 secid, struct aa_label *label) +{ + struct aa_label *old; + unsigned long flags; + + spin_lock_irqsave(&secid_lock, flags); + old = __secid_update(secid, label); + spin_unlock_irqrestore(&secid_lock, flags); +} + +/** + * + * see label for inverse aa_label_to_secid + */ +struct aa_label *aa_secid_to_label(u32 secid) +{ + struct aa_label *label; + + rcu_read_lock(); + label = radix_tree_lookup(&aa_secids_map, secid); + rcu_read_unlock(); + + return label; +} + +int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen) +{ + /* TODO: cache secctx and ref count so we don't have to recreate */ + struct aa_label *label = aa_secid_to_label(secid); + + AA_BUG(!secdata); + AA_BUG(!seclen); + + if (!label) + return -EINVAL; + + if (secdata) + *seclen = aa_label_asxprint(secdata, root_ns, label, + FLAG_SHOW_MODE | FLAG_VIEW_SUBNS | + FLAG_HIDDEN_UNCONFINED | + FLAG_ABS_ROOT, GFP_ATOMIC); + else + *seclen = aa_label_snxprint(NULL, 0, root_ns, label, + FLAG_SHOW_MODE | FLAG_VIEW_SUBNS | + FLAG_HIDDEN_UNCONFINED | + FLAG_ABS_ROOT); + if (*seclen < 0) + return -ENOMEM; + + return 0; +} + + +int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid) +{ + struct aa_label *label; + + label = aa_label_strn_parse(&root_ns->unconfined->label, secdata, + seclen, GFP_KERNEL, false, false); + if (IS_ERR(label)) + return PTR_ERR(label); + *secid = label->secid; + + return 0; +} + +void apparmor_release_secctx(char *secdata, u32 seclen) +{ + kfree(secdata); +} -/* TODO FIXME: add secid to profile mapping, and secid recycling */ /** * aa_alloc_secid - allocate a new secid for a profile */ -u32 aa_alloc_secid(void) +u32 aa_alloc_secid(struct aa_label *label, gfp_t gfp) { + unsigned long flags; u32 secid; - /* - * TODO FIXME: secid recycling - part of profile mapping table - */ - spin_lock(&secid_lock); - secid = (++global_secid); - spin_unlock(&secid_lock); + /* racey, but at worst causes new allocation instead of reuse */ + if (free_list == FREE_LIST_HEAD) { + bool preload = 0; + int res; + +retry: + if (gfpflags_allow_blocking(gfp) && !radix_tree_preload(gfp)) + preload = 1; + spin_lock_irqsave(&secid_lock, flags); + if (alloced_secid != SECID_MAX) { + secid = ++alloced_secid; + res = radix_tree_insert(&aa_secids_map, secid, label); + AA_BUG(res == -EEXIST); + } else { + secid = AA_SECID_INVALID; + } + spin_unlock_irqrestore(&secid_lock, flags); + if (preload) + radix_tree_preload_end(); + } else { + spin_lock_irqsave(&secid_lock, flags); + /* remove entry from free list */ + secid = __pop(label); + if (secid == AA_SECID_INVALID) { + spin_unlock_irqrestore(&secid_lock, flags); + goto retry; + } + spin_unlock_irqrestore(&secid_lock, flags); + } + return secid; } @@ -51,5 +232,9 @@ u32 aa_alloc_secid(void) */ void aa_free_secid(u32 secid) { - ; /* NOP ATM */ + unsigned long flags; + + spin_lock_irqsave(&secid_lock, flags); + __push(secid); + spin_unlock_irqrestore(&secid_lock, flags); } -- cgit v1.2.3-59-g8ed1b From a7ae3645f5cf3f0cb2420522b7b3ff2352bb1ee8 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 11 Sep 2017 11:29:53 -0700 Subject: apparmor: add the ability to get a task's secid Signed-off-by: John Johansen --- security/apparmor/lsm.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'security') diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 91284b5d56a3..7866161f685b 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -711,6 +711,13 @@ static void apparmor_bprm_committed_creds(struct linux_binprm *bprm) return; } +static void apparmor_task_getsecid(struct task_struct *p, u32 *secid) +{ + struct aa_label *label = aa_get_task_label(p); + *secid = label->secid; + aa_put_label(label); +} + static int apparmor_task_setrlimit(struct task_struct *task, unsigned int resource, struct rlimit *new_rlim) { @@ -1187,6 +1194,7 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(task_free, apparmor_task_free), LSM_HOOK_INIT(task_alloc, apparmor_task_alloc), + LSM_HOOK_INIT(task_getsecid, apparmor_task_getsecid), LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit), LSM_HOOK_INIT(task_kill, apparmor_task_kill), -- cgit v1.2.3-59-g8ed1b From b2c2086c3984d96045e758d984d859caae6f96ca Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Thu, 12 Apr 2018 12:34:28 +0200 Subject: apparmor: fix typo "loosen" Signed-off-by: Zygmunt Krynicki Acked-by: Christian Boltz Signed-off-by: John Johansen --- security/apparmor/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 590b7e8cd21c..098d546d8253 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -839,7 +839,7 @@ static struct aa_label *handle_onexec(struct aa_label *label, cond, unsafe)); } else { - /* TODO: determine how much we want to losen this */ + /* TODO: determine how much we want to loosen this */ error = fn_for_each_in_ns(label, profile, profile_onexec(profile, onexec, stack, bprm, buffer, cond, unsafe)); -- cgit v1.2.3-59-g8ed1b From a18f902888c017478009d3cd17fd3f5ed1dcbc43 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Thu, 12 Apr 2018 12:34:29 +0200 Subject: apparmor: fix typo "comparison" Signed-off-by: Zygmunt Krynicki Acked-by: Christian Boltz Signed-off-by: John Johansen --- security/apparmor/label.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 152352755869..fa3f17bbe9e4 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -128,7 +128,7 @@ static int ns_cmp(struct aa_ns *a, struct aa_ns *b) } /** - * profile_cmp - profile comparision for set ordering + * profile_cmp - profile comparison for set ordering * @a: profile to compare (NOT NULL) * @b: profile to compare (NOT NULL) * @@ -157,7 +157,7 @@ static int profile_cmp(struct aa_profile *a, struct aa_profile *b) } /** - * vec_cmp - label comparision for set ordering + * vec_cmp - label comparison for set ordering * @a: label to compare (NOT NULL) * @vec: vector of profiles to compare (NOT NULL) * @n: length of @vec @@ -463,7 +463,7 @@ fail: /** - * label_cmp - label comparision for set ordering + * label_cmp - label comparison for set ordering * @a: label to compare (NOT NULL) * @b: label to compare (NOT NULL) * -- cgit v1.2.3-59-g8ed1b From b62fb22674dd7c7eb7e8aeec5937165e8b063f9c Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Thu, 12 Apr 2018 12:34:30 +0200 Subject: apparmor: fix typo "replace" Signed-off-by: Zygmunt Krynicki Acked-by: Christian Boltz Signed-off-by: John Johansen --- security/apparmor/label.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/label.c b/security/apparmor/label.c index fa3f17bbe9e4..a17574df611b 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -2011,7 +2011,7 @@ out: /** * __label_update - insert updated version of @label into labelset - * @label - the label to update/repace + * @label - the label to update/replace * * Returns: new label that is up to date * else NULL on failure -- cgit v1.2.3-59-g8ed1b From 69ad4a44a26ee8fefdb75848ccb6784f5254f14c Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Thu, 12 Apr 2018 12:34:31 +0200 Subject: apparmor: fix typo "type" Signed-off-by: Zygmunt Krynicki Acked-by: Christian Boltz Signed-off-by: John Johansen --- security/apparmor/lib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index 068a9f471f77..a7b3f681b80e 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -408,7 +408,7 @@ int aa_profile_label_perm(struct aa_profile *profile, struct aa_profile *target, * @request: requested perms * @deny: Returns: explicit deny set * @sa: initialized audit structure (MAY BE NULL if not auditing) - * @cb: callback fn for tpye specific fields (MAY BE NULL) + * @cb: callback fn for type specific fields (MAY BE NULL) * * Returns: 0 if permission else error code * -- cgit v1.2.3-59-g8ed1b From 5d2371e1235b6852ff606db076ebc7abee48a5a4 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Thu, 12 Apr 2018 12:34:32 +0200 Subject: apparmor: fix typo "traverse" Signed-off-by: Zygmunt Krynicki Acked-by: Christian Boltz Signed-off-by: John Johansen --- security/apparmor/match.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 280eba082c7b..55f2ee505a01 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -472,7 +472,7 @@ unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start, /** * aa_dfa_next - step one character to the next state in the dfa - * @dfa: the dfa to tranverse (NOT NULL) + * @dfa: the dfa to traverse (NOT NULL) * @state: the state to start in * @c: the input character to transition on * -- cgit v1.2.3-59-g8ed1b From 68a1a0c68c6ecc84ab04f82ca0eac3f8a495a0a8 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Thu, 12 Apr 2018 12:34:33 +0200 Subject: apparmor: fix typo "independent" Signed-off-by: Zygmunt Krynicki Acked-by: Christian Boltz Signed-off-by: John Johansen --- security/apparmor/mount.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c index 6e8c7ac0b33d..c1da22482bfb 100644 --- a/security/apparmor/mount.c +++ b/security/apparmor/mount.c @@ -121,7 +121,7 @@ static void audit_cb(struct audit_buffer *ab, void *va) * @src_name: src_name of object being mediated (MAYBE_NULL) * @type: type of filesystem (MAYBE_NULL) * @trans: name of trans (MAYBE NULL) - * @flags: filesystem idependent mount flags + * @flags: filesystem independent mount flags * @data: filesystem mount flags * @request: permissions requested * @perms: the permissions computed for the request (NOT NULL) -- cgit v1.2.3-59-g8ed1b From 3107e8cb9219cff359b93dde257c030b500e74b7 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Thu, 12 Apr 2018 12:34:34 +0200 Subject: apparmor: fix typo "preconfinement" Signed-off-by: Zygmunt Krynicki Acked-by: Christian Boltz Signed-off-by: John Johansen --- security/apparmor/policy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index d68252b112dc..b367fef33d03 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -1085,7 +1085,7 @@ fail: * Remove a profile or sub namespace from the current namespace, so that * they can not be found anymore and mark them as replaced by unconfined * - * NOTE: removing confinement does not restore rlimits to preconfinemnet values + * NOTE: removing confinement does not restore rlimits to preconfinement values * * Returns: size of data consume else error code if fails */ -- cgit v1.2.3-59-g8ed1b From 52e7128ebbdd7b05ba8615efbe410e88a5925a1d Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 4 May 2018 01:57:47 -0700 Subject: apparmor: fix '*seclen' is never less than zero smatch warnings: security/apparmor/secid.c:162 apparmor_secid_to_secctx() warn: unsigned '*seclen' is never less than zero. vim +162 security/apparmor/secid.c 140 141 int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen) 142 { 143 /* TODO: cache secctx and ref count so we don't have to recreate */ 144 struct aa_label *label = aa_secid_to_label(secid); 145 146 AA_BUG(!secdata); 147 AA_BUG(!seclen); 148 149 if (!label) 150 return -EINVAL; 151 152 if (secdata) 153 *seclen = aa_label_asxprint(secdata, root_ns, label, 154 FLAG_SHOW_MODE | FLAG_VIEW_SUBNS | 155 FLAG_HIDDEN_UNCONFINED | 156 FLAG_ABS_ROOT, GFP_ATOMIC); 157 else 158 *seclen = aa_label_snxprint(NULL, 0, root_ns, label, 159 FLAG_SHOW_MODE | FLAG_VIEW_SUBNS | 160 FLAG_HIDDEN_UNCONFINED | 161 FLAG_ABS_ROOT); > 162 if (*seclen < 0) 163 return -ENOMEM; 164 165 return 0; 166 } 167 Fixes: c092921219d2 ("apparmor: add support for mapping secids and using secctxes") Signed-off-by: John Johansen --- security/apparmor/secid.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c index 502924853986..c2f0c1571156 100644 --- a/security/apparmor/secid.c +++ b/security/apparmor/secid.c @@ -142,6 +142,7 @@ int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen) { /* TODO: cache secctx and ref count so we don't have to recreate */ struct aa_label *label = aa_secid_to_label(secid); + int len; AA_BUG(!secdata); AA_BUG(!seclen); @@ -150,18 +151,19 @@ int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen) return -EINVAL; if (secdata) - *seclen = aa_label_asxprint(secdata, root_ns, label, - FLAG_SHOW_MODE | FLAG_VIEW_SUBNS | - FLAG_HIDDEN_UNCONFINED | - FLAG_ABS_ROOT, GFP_ATOMIC); + len = aa_label_asxprint(secdata, root_ns, label, + FLAG_SHOW_MODE | FLAG_VIEW_SUBNS | + FLAG_HIDDEN_UNCONFINED | FLAG_ABS_ROOT, + GFP_ATOMIC); else - *seclen = aa_label_snxprint(NULL, 0, root_ns, label, - FLAG_SHOW_MODE | FLAG_VIEW_SUBNS | - FLAG_HIDDEN_UNCONFINED | - FLAG_ABS_ROOT); - if (*seclen < 0) + len = aa_label_snxprint(NULL, 0, root_ns, label, + FLAG_SHOW_MODE | FLAG_VIEW_SUBNS | + FLAG_HIDDEN_UNCONFINED | FLAG_ABS_ROOT); + if (len < 0) return -ENOMEM; + *seclen = len; + return 0; } -- cgit v1.2.3-59-g8ed1b From 38125c2c2beb3c770d8fcdbcd846bd95938866d3 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 4 May 2018 02:18:07 -0700 Subject: apparmor: improve get_buffers macro by using get_cpu_ptr Refactor get_buffers so the cpu_ptr can be obtained in the outer layer, instead of inside the macro. This also enables us to cleanup the code and use get_cpu_ptr, to handle the preempt_disable() Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/include/path.h | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/path.h b/security/apparmor/include/path.h index e042b994f2b8..b6380c5f0097 100644 --- a/security/apparmor/include/path.h +++ b/security/apparmor/include/path.h @@ -43,10 +43,11 @@ struct aa_buffers { DECLARE_PER_CPU(struct aa_buffers, aa_buffers); -#define ASSIGN(FN, X, N) ((X) = FN(N)) -#define EVAL1(FN, X) ASSIGN(FN, X, 0) /*X = FN(0)*/ -#define EVAL2(FN, X, Y...) do { ASSIGN(FN, X, 1); EVAL1(FN, Y); } while (0) -#define EVAL(FN, X...) CONCATENATE(EVAL, COUNT_ARGS(X))(FN, X) +#define ASSIGN(FN, A, X, N) ((X) = FN(A, N)) +#define EVAL1(FN, A, X) ASSIGN(FN, A, X, 0) /*X = FN(0)*/ +#define EVAL2(FN, A, X, Y...) \ + do { ASSIGN(FN, A, X, 1); EVAL1(FN, A, Y); } while (0) +#define EVAL(FN, A, X...) CONCATENATE(EVAL, COUNT_ARGS(X))(FN, A, X) #define for_each_cpu_buffer(I) for ((I) = 0; (I) < MAX_PATH_BUFFERS; (I)++) @@ -56,26 +57,24 @@ DECLARE_PER_CPU(struct aa_buffers, aa_buffers); #define AA_BUG_PREEMPT_ENABLED(X) /* nop */ #endif -#define __get_buffer(N) ({ \ - struct aa_buffers *__cpu_var; \ +#define __get_buffer(C, N) ({ \ AA_BUG_PREEMPT_ENABLED("__get_buffer without preempt disabled"); \ - __cpu_var = this_cpu_ptr(&aa_buffers); \ - __cpu_var->buf[(N)]; }) + (C)->buf[(N)]; }) -#define __get_buffers(X...) EVAL(__get_buffer, X) +#define __get_buffers(C, X...) EVAL(__get_buffer, C, X) #define __put_buffers(X, Y...) ((void)&(X)) -#define get_buffers(X...) \ -do { \ - preempt_disable(); \ - __get_buffers(X); \ +#define get_buffers(X...) \ +do { \ + struct aa_buffers *__cpu_var = get_cpu_ptr(&aa_buffers); \ + __get_buffers(__cpu_var, X); \ } while (0) -#define put_buffers(X, Y...) \ -do { \ - __put_buffers(X, Y); \ - preempt_enable(); \ +#define put_buffers(X, Y...) \ +do { \ + __put_buffers(X, Y); \ + put_cpu_ptr(&aa_buffers); \ } while (0) #endif /* __AA_PATH_H */ -- cgit v1.2.3-59-g8ed1b From 5d8779a5cdda5530d5706586638a5cf0ac5bd8a3 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 7 May 2018 16:39:03 +0300 Subject: apparmor: Convert to use match_string() helper The new helper returns index of the matching string in an array. We are going to use it here. Signed-off-by: Andy Shevchenko Reviewed-by: Jay Freyensee Signed-off-by: John Johansen --- security/apparmor/lsm.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) (limited to 'security') diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 7866161f685b..8299a5d13fee 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1391,14 +1391,12 @@ static int param_set_audit(const char *val, const struct kernel_param *kp) if (apparmor_initialized && !policy_admin_capable(NULL)) return -EPERM; - for (i = 0; i < AUDIT_MAX_INDEX; i++) { - if (strcmp(val, audit_mode_names[i]) == 0) { - aa_g_audit = i; - return 0; - } - } + i = match_string(audit_mode_names, AUDIT_MAX_INDEX, val); + if (i < 0) + return -EINVAL; - return -EINVAL; + aa_g_audit = i; + return 0; } static int param_get_mode(char *buffer, const struct kernel_param *kp) @@ -1422,14 +1420,13 @@ static int param_set_mode(const char *val, const struct kernel_param *kp) if (apparmor_initialized && !policy_admin_capable(NULL)) return -EPERM; - 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; - } - } + i = match_string(aa_profile_mode_names, APPARMOR_MODE_NAMES_MAX_INDEX, + val); + if (i < 0) + return -EINVAL; - return -EINVAL; + aa_g_profile_mode = i; + return 0; } /* -- cgit v1.2.3-59-g8ed1b From e79c26d04043b15de64f082d4da52e9fff7ca607 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Mon, 16 Apr 2018 11:23:58 -0700 Subject: apparmor: Add support for audit rule filtering This patch adds support to Apparmor for integrating with audit rule filtering. Right now it only handles SUBJ_ROLE, interpreting it as a single component of a label. This is sufficient to get Apparmor working with IMA's appraisal rules without any modifications on the IMA side. Signed-off-by: Matthew Garrett Signed-off-by: John Johansen --- security/apparmor/audit.c | 95 ++++++++++++++++++++++++++++++++++++++- security/apparmor/include/audit.h | 6 +++ security/apparmor/lsm.c | 7 +++ 3 files changed, 107 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c index 8f9ecac7f8de..7ac7c8190cc4 100644 --- a/security/apparmor/audit.c +++ b/security/apparmor/audit.c @@ -19,7 +19,7 @@ #include "include/audit.h" #include "include/policy.h" #include "include/policy_ns.h" - +#include "include/secid.h" const char *const audit_mode_names[] = { "normal", @@ -163,3 +163,96 @@ int aa_audit(int type, struct aa_profile *profile, struct common_audit_data *sa, return aad(sa)->error; } + +struct aa_audit_rule { + char *profile; +}; + +void aa_audit_rule_free(void *vrule) +{ + struct aa_audit_rule *rule = vrule; + + if (rule) { + kfree(rule->profile); + kfree(rule); + } +} + +int aa_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) +{ + struct aa_audit_rule *rule; + + switch (field) { + case AUDIT_SUBJ_ROLE: + if (op != Audit_equal && op != Audit_not_equal) + return -EINVAL; + break; + default: + return -EINVAL; + } + + rule = kzalloc(sizeof(struct aa_audit_rule), GFP_KERNEL); + + if (!rule) + return -ENOMEM; + + rule->profile = kstrdup(rulestr, GFP_KERNEL); + + if (!rule->profile) { + kfree(rule); + return -ENOMEM; + } + + *vrule = rule; + + return 0; +} + +int aa_audit_rule_known(struct audit_krule *rule) +{ + int i; + + for (i = 0; i < rule->field_count; i++) { + struct audit_field *f = &rule->fields[i]; + + switch (f->type) { + case AUDIT_SUBJ_ROLE: + return 1; + } + } + + return 0; +} + +int aa_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule, + struct audit_context *actx) +{ + struct aa_audit_rule *rule = vrule; + struct aa_label *label; + struct label_it i; + struct aa_profile *profile; + int found = 0; + + label = aa_secid_to_label(sid); + + if (!label) + return -ENOENT; + + label_for_each(i, label, profile) { + if (strcmp(rule->profile, profile->base.hname) == 0) { + found = 1; + break; + } + } + + switch (field) { + case AUDIT_SUBJ_ROLE: + switch (op) { + case Audit_equal: + return found; + case Audit_not_equal: + return !found; + } + } + return 0; +} diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h index 9c9be9c98c15..b8c8b1066b0a 100644 --- a/security/apparmor/include/audit.h +++ b/security/apparmor/include/audit.h @@ -189,4 +189,10 @@ static inline int complain_error(int error) return error; } +void aa_audit_rule_free(void *vrule); +int aa_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule); +int aa_audit_rule_known(struct audit_krule *rule); +int aa_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule, + struct audit_context *actx); + #endif /* __AA_AUDIT_H */ diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 8299a5d13fee..10bf36aa477d 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1198,6 +1198,13 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit), LSM_HOOK_INIT(task_kill, apparmor_task_kill), +#ifdef CONFIG_AUDIT + LSM_HOOK_INIT(audit_rule_init, aa_audit_rule_init), + LSM_HOOK_INIT(audit_rule_known, aa_audit_rule_known), + LSM_HOOK_INIT(audit_rule_match, aa_audit_rule_match), + LSM_HOOK_INIT(audit_rule_free, aa_audit_rule_free), +#endif + LSM_HOOK_INIT(secid_to_secctx, apparmor_secid_to_secctx), LSM_HOOK_INIT(secctx_to_secid, apparmor_secctx_to_secid), LSM_HOOK_INIT(release_secctx, apparmor_release_secctx), -- cgit v1.2.3-59-g8ed1b From 2ab47dae54d567bbb1ad3e96e5b2601cc13f4d2b Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 3 May 2018 00:39:58 -0700 Subject: apparmor: modify audit rule support to support profile stacks Allows for audit rules, where a rule could specify a profile stack A//&B, while extending the current semantic so if the label specified in the audit rule is a subset of the secid it is considered a match. Eg. if the secid resolves to the label stack A//&B//&C Then an audit rule specifying a label of A - would match B - would match C - would match D - would not A//&B - would match as a subset A//&C - would match as a subset B//&C - would match as a subset A//&B//&C - would match A//&D - would not match, because while A does match, D is also specified and does not Note: audit rules are currently assumed to be coming from the root namespace. Signed-off-by: John Johansen --- security/apparmor/audit.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) (limited to 'security') diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c index 7ac7c8190cc4..575f3e9c8c80 100644 --- a/security/apparmor/audit.c +++ b/security/apparmor/audit.c @@ -165,7 +165,7 @@ int aa_audit(int type, struct aa_profile *profile, struct common_audit_data *sa, } struct aa_audit_rule { - char *profile; + struct aa_label *label; }; void aa_audit_rule_free(void *vrule) @@ -173,7 +173,8 @@ void aa_audit_rule_free(void *vrule) struct aa_audit_rule *rule = vrule; if (rule) { - kfree(rule->profile); + if (!IS_ERR(rule->label)) + aa_put_label(rule->label); kfree(rule); } } @@ -196,13 +197,11 @@ int aa_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) if (!rule) return -ENOMEM; - rule->profile = kstrdup(rulestr, GFP_KERNEL); - - if (!rule->profile) { - kfree(rule); - return -ENOMEM; - } - + /* Currently rules are treated as coming from the root ns */ + rule->label = aa_label_parse(&root_ns->unconfined->label, rulestr, + GFP_KERNEL, true, false); + if (IS_ERR(rule->label)) + return PTR_ERR(rule->label); *vrule = rule; return 0; @@ -229,8 +228,6 @@ int aa_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule, { struct aa_audit_rule *rule = vrule; struct aa_label *label; - struct label_it i; - struct aa_profile *profile; int found = 0; label = aa_secid_to_label(sid); @@ -238,12 +235,8 @@ int aa_audit_rule_match(u32 sid, u32 field, u32 op, void *vrule, if (!label) return -ENOENT; - label_for_each(i, label, profile) { - if (strcmp(rule->profile, profile->base.hname) == 0) { - found = 1; - break; - } - } + if (aa_label_is_subset(label, rule->label)) + found = 1; switch (field) { case AUDIT_SUBJ_ROLE: -- cgit v1.2.3-59-g8ed1b From 52e8c38001d8ef0ca07ef428e480cd4a35e46abf Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Thu, 17 May 2018 19:53:45 +0000 Subject: apparmor: Fix memory leak of rule on error exit path Currently on the error exit path the allocated rule is not free'd causing a memory leak. Fix this by calling aa_audit_rule_free(). Detected by CoverityScan, CID#1468966 ("Resource leaks") Fixes: cb740f574c7b ("apparmor: modify audit rule support to support profile stacks") Signed-off-by: Tyler Hicks Signed-off-by: John Johansen --- security/apparmor/audit.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c index 575f3e9c8c80..eeaddfe0c0fb 100644 --- a/security/apparmor/audit.c +++ b/security/apparmor/audit.c @@ -200,10 +200,12 @@ int aa_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) /* Currently rules are treated as coming from the root ns */ rule->label = aa_label_parse(&root_ns->unconfined->label, rulestr, GFP_KERNEL, true, false); - if (IS_ERR(rule->label)) + if (IS_ERR(rule->label)) { + aa_audit_rule_free(rule); return PTR_ERR(rule->label); - *vrule = rule; + } + *vrule = rule; return 0; } -- cgit v1.2.3-59-g8ed1b From 99cc45e486786c7215a7e39824c3bbaf7cf2fc08 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Tue, 22 May 2018 02:32:59 -0700 Subject: apparmor: Use an IDR to allocate apparmor secids Replace the custom usage of the radix tree to store a list of free IDs with the IDR. Signed-off-by: Matthew Wilcox Signed-off-by: John Johansen --- security/apparmor/secid.c | 114 +++++----------------------------------------- 1 file changed, 11 insertions(+), 103 deletions(-) (limited to 'security') diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c index c2f0c1571156..3ad94b2ffbb2 100644 --- a/security/apparmor/secid.c +++ b/security/apparmor/secid.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -30,18 +31,10 @@ /* * secids - do not pin labels with a refcount. They rely on the label * properly updating/freeing them - * - * A singly linked free list is used to track secids that have been - * freed and reuse them before allocating new ones */ -#define FREE_LIST_HEAD 1 - -static RADIX_TREE(aa_secids_map, GFP_ATOMIC); +static DEFINE_IDR(aa_secids); static DEFINE_SPINLOCK(secid_lock); -static u32 alloced_secid = FREE_LIST_HEAD; -static u32 free_list = FREE_LIST_HEAD; -static unsigned long free_count; /* * TODO: allow policy to reserve a secid range? @@ -49,65 +42,6 @@ static unsigned long free_count; * TODO: use secid_update in label replace */ -#define SECID_MAX U32_MAX - -/* TODO: mark free list as exceptional */ -static void *to_ptr(u32 secid) -{ - return (void *) - ((((unsigned long) secid) << RADIX_TREE_EXCEPTIONAL_SHIFT)); -} - -static u32 to_secid(void *ptr) -{ - return (u32) (((unsigned long) ptr) >> RADIX_TREE_EXCEPTIONAL_SHIFT); -} - - -/* TODO: tag free_list entries to mark them as different */ -static u32 __pop(struct aa_label *label) -{ - u32 secid = free_list; - void __rcu **slot; - void *entry; - - if (free_list == FREE_LIST_HEAD) - return AA_SECID_INVALID; - - slot = radix_tree_lookup_slot(&aa_secids_map, secid); - AA_BUG(!slot); - entry = radix_tree_deref_slot_protected(slot, &secid_lock); - free_list = to_secid(entry); - radix_tree_replace_slot(&aa_secids_map, slot, label); - free_count--; - - return secid; -} - -static void __push(u32 secid) -{ - void __rcu **slot; - - slot = radix_tree_lookup_slot(&aa_secids_map, secid); - AA_BUG(!slot); - radix_tree_replace_slot(&aa_secids_map, slot, to_ptr(free_list)); - free_list = secid; - free_count++; -} - -static struct aa_label * __secid_update(u32 secid, struct aa_label *label) -{ - struct aa_label *old; - void __rcu **slot; - - slot = radix_tree_lookup_slot(&aa_secids_map, secid); - AA_BUG(!slot); - old = radix_tree_deref_slot_protected(slot, &secid_lock); - radix_tree_replace_slot(&aa_secids_map, slot, label); - - return old; -} - /** * aa_secid_update - update a secid mapping to a new label * @secid: secid to update @@ -115,11 +49,10 @@ static struct aa_label * __secid_update(u32 secid, struct aa_label *label) */ void aa_secid_update(u32 secid, struct aa_label *label) { - struct aa_label *old; unsigned long flags; spin_lock_irqsave(&secid_lock, flags); - old = __secid_update(secid, label); + idr_replace(&aa_secids, label, secid); spin_unlock_irqrestore(&secid_lock, flags); } @@ -132,7 +65,7 @@ struct aa_label *aa_secid_to_label(u32 secid) struct aa_label *label; rcu_read_lock(); - label = radix_tree_lookup(&aa_secids_map, secid); + label = idr_find(&aa_secids, secid); rcu_read_unlock(); return label; @@ -167,7 +100,6 @@ int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen) return 0; } - int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid) { struct aa_label *label; @@ -186,7 +118,6 @@ void apparmor_release_secctx(char *secdata, u32 seclen) kfree(secdata); } - /** * aa_alloc_secid - allocate a new secid for a profile */ @@ -195,35 +126,12 @@ u32 aa_alloc_secid(struct aa_label *label, gfp_t gfp) unsigned long flags; u32 secid; - /* racey, but at worst causes new allocation instead of reuse */ - if (free_list == FREE_LIST_HEAD) { - bool preload = 0; - int res; - -retry: - if (gfpflags_allow_blocking(gfp) && !radix_tree_preload(gfp)) - preload = 1; - spin_lock_irqsave(&secid_lock, flags); - if (alloced_secid != SECID_MAX) { - secid = ++alloced_secid; - res = radix_tree_insert(&aa_secids_map, secid, label); - AA_BUG(res == -EEXIST); - } else { - secid = AA_SECID_INVALID; - } - spin_unlock_irqrestore(&secid_lock, flags); - if (preload) - radix_tree_preload_end(); - } else { - spin_lock_irqsave(&secid_lock, flags); - /* remove entry from free list */ - secid = __pop(label); - if (secid == AA_SECID_INVALID) { - spin_unlock_irqrestore(&secid_lock, flags); - goto retry; - } - spin_unlock_irqrestore(&secid_lock, flags); - } + idr_preload(gfp); + spin_lock_irqsave(&secid_lock, flags); + secid = idr_alloc(&aa_secids, label, 0, 0, GFP_ATOMIC); + /* XXX: Can return -ENOMEM */ + spin_unlock_irqrestore(&secid_lock, flags); + idr_preload_end(); return secid; } @@ -237,6 +145,6 @@ void aa_free_secid(u32 secid) unsigned long flags; spin_lock_irqsave(&secid_lock, flags); - __push(secid); + idr_remove(&aa_secids, secid); spin_unlock_irqrestore(&secid_lock, flags); } -- cgit v1.2.3-59-g8ed1b From a4c3f89c9b5a9fab5a8e4ea05399acd6e23072df Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 4 Jun 2018 19:44:59 -0700 Subject: apparmor: fixup secid map conversion to using IDR The IDR conversion did not handle an error case for when allocating a mapping fails, and it did not ensure that mappings did not allocate or use a 0 value, which is used as an invalid secid. Which is used when a mapping fails. Fixes: 3ae7eb49a2be ("apparmor: Use an IDR to allocate apparmor secids") Signed-off-by: John Johansen --- security/apparmor/include/secid.h | 4 +++- security/apparmor/label.c | 3 +-- security/apparmor/lsm.c | 2 ++ security/apparmor/secid.c | 28 +++++++++++++++++++++++----- 4 files changed, 29 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/secid.h b/security/apparmor/include/secid.h index 686de8e50a79..dee6fa3b6081 100644 --- a/security/apparmor/include/secid.h +++ b/security/apparmor/include/secid.h @@ -28,8 +28,10 @@ int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid); void apparmor_release_secctx(char *secdata, u32 seclen); -u32 aa_alloc_secid(struct aa_label *label, gfp_t gfp); +int aa_alloc_secid(struct aa_label *label, gfp_t gfp); void aa_free_secid(u32 secid); void aa_secid_update(u32 secid, struct aa_label *label); +void aa_secids_init(void); + #endif /* __AA_SECID_H */ diff --git a/security/apparmor/label.c b/security/apparmor/label.c index a17574df611b..ba11bdf9043a 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -407,8 +407,7 @@ bool aa_label_init(struct aa_label *label, int size, gfp_t gfp) AA_BUG(!label); AA_BUG(size < 1); - label->secid = aa_alloc_secid(label, gfp); - if (label->secid == AA_SECID_INVALID) + if (aa_alloc_secid(label, gfp) < 0) return false; label->size = size; /* doesn't include null */ diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 10bf36aa477d..e35d12883990 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1547,6 +1547,8 @@ static int __init apparmor_init(void) return 0; } + aa_secids_init(); + error = aa_setup_dfa_engine(); if (error) { AA_ERROR("Unable to setup dfa engine\n"); diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c index 3ad94b2ffbb2..f2f22d00db18 100644 --- a/security/apparmor/secid.c +++ b/security/apparmor/secid.c @@ -33,6 +33,8 @@ * properly updating/freeing them */ +#define AA_FIRST_SECID 1 + static DEFINE_IDR(aa_secids); static DEFINE_SPINLOCK(secid_lock); @@ -120,20 +122,31 @@ void apparmor_release_secctx(char *secdata, u32 seclen) /** * aa_alloc_secid - allocate a new secid for a profile + * @label: the label to allocate a secid for + * @gfp: memory allocation flags + * + * Returns: 0 with @label->secid initialized + * <0 returns error with @label->secid set to AA_SECID_INVALID */ -u32 aa_alloc_secid(struct aa_label *label, gfp_t gfp) +int aa_alloc_secid(struct aa_label *label, gfp_t gfp) { unsigned long flags; - u32 secid; + int ret; idr_preload(gfp); spin_lock_irqsave(&secid_lock, flags); - secid = idr_alloc(&aa_secids, label, 0, 0, GFP_ATOMIC); - /* XXX: Can return -ENOMEM */ + ret = idr_alloc(&aa_secids, label, AA_FIRST_SECID, 0, GFP_ATOMIC); spin_unlock_irqrestore(&secid_lock, flags); idr_preload_end(); - return secid; + if (ret < 0) { + label->secid = AA_SECID_INVALID; + return ret; + } + + AA_BUG(ret == AA_SECID_INVALID); + label->secid = ret; + return 0; } /** @@ -148,3 +161,8 @@ void aa_free_secid(u32 secid) idr_remove(&aa_secids, secid); spin_unlock_irqrestore(&secid_lock, flags); } + +void aa_secids_init(void) +{ + idr_init_base(&aa_secids, AA_FIRST_SECID); +} -- cgit v1.2.3-59-g8ed1b From 11c92f144bf39f448f65202cccba672097a1100b Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 11 Apr 2018 02:03:26 -0700 Subject: apparmor: fix mediation of prlimit For primit apparmor requires that if target confinement does not match the setting task's confinement, the setting task requires CAP_SYS_RESOURCE. Unfortunately this was broken when rlimit enforcement was reworked to support labels. Fixes: 86b92cb782b3 ("apparmor: move resource checks to using labels") Signed-off-by: John Johansen --- security/apparmor/resource.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/resource.c b/security/apparmor/resource.c index d022137143b9..95fd26d09757 100644 --- a/security/apparmor/resource.c +++ b/security/apparmor/resource.c @@ -124,7 +124,7 @@ int aa_task_setrlimit(struct aa_label *label, struct task_struct *task, */ if (label != peer && - !aa_capable(label, CAP_SYS_RESOURCE, SECURITY_CAP_NOAUDIT)) + aa_capable(label, CAP_SYS_RESOURCE, SECURITY_CAP_NOAUDIT) != 0) error = fn_for_each(label, profile, audit_resource(profile, resource, new_rlim->rlim_max, peer, -- cgit v1.2.3-59-g8ed1b From 3ddae9876a7045a8d08ab372eff232a5da5199b8 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 13 Apr 2018 22:33:10 -0700 Subject: apparmor: fix memory leak when deduping profile load AppArmor is leaking the newly loaded profile and its proxy when the profile is an exact match to the currently loaded version. In this case the dedup check results in the profile being skipped and put without dealing with the proxy ref thus not breaking a circular refcount and causing a leak. BugLink: http://bugs.launchpad.net/bugs/1750594 Fixes: 5d5182cae401 ("apparmor: move to per loaddata files, instead of replicating in profiles") Signed-off-by: John Johansen --- security/apparmor/policy.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'security') diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index b367fef33d03..1590e2de4e84 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -1008,6 +1008,9 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label, audit_policy(label, op, ns_name, ent->new->base.hname, "same as current profile, skipping", error); + /* break refcount cycle with proxy. */ + aa_put_proxy(ent->new->label.proxy); + ent->new->label.proxy = NULL; goto skip; } -- cgit v1.2.3-59-g8ed1b From 338d0be437ef10e247a35aed83dbab182cf406a2 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 7 Jun 2018 00:45:30 -0700 Subject: apparmor: fix ptrace read check The ptrace read check is incorrect resulting in policy that is broader than it needs to be. Fix the check so that read access permission can be properly detected when other ptrace flags are set. Fixes: b2d09ae449ce ("apparmor: move ptrace checks to using labels") Signed-off-by: John Johansen --- security/apparmor/lsm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index e35d12883990..74f17376202b 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -117,7 +117,8 @@ static int apparmor_ptrace_access_check(struct task_struct *child, tracer = begin_current_label_crit_section(); tracee = aa_get_task_label(child); error = aa_may_ptrace(tracer, tracee, - mode == PTRACE_MODE_READ ? AA_PTRACE_READ : AA_PTRACE_TRACE); + (mode & PTRACE_MODE_READ) ? AA_PTRACE_READ + : AA_PTRACE_TRACE); aa_put_label(tracee); end_current_label_crit_section(tracer); -- cgit v1.2.3-59-g8ed1b