From 26898fdff371d78f122cf15d8732d1d37f2d1338 Mon Sep 17 00:00:00 2001 From: Aristeu Rozanski Date: Fri, 15 Feb 2013 11:55:44 -0500 Subject: devcg: expand may_access() logic In order to make the next patch more clear, expand may_access() logic. v2: may_access() returns bool now Acked-by: Tejun Heo Acked-by: Serge Hallyn Cc: Tejun Heo Cc: Serge Hallyn Signed-off-by: Aristeu Rozanski Signed-off-by: Tejun Heo --- security/device_cgroup.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 1c69e38e3a2..4ad55a9c692 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -305,8 +305,8 @@ static int devcgroup_seq_read(struct cgroup *cgroup, struct cftype *cft, * @dev_cgroup: dev cgroup to be tested against * @refex: new exception */ -static int may_access(struct dev_cgroup *dev_cgroup, - struct dev_exception_item *refex) +static bool may_access(struct dev_cgroup *dev_cgroup, + struct dev_exception_item *refex) { struct dev_exception_item *ex; bool match = false; @@ -332,16 +332,19 @@ static int may_access(struct dev_cgroup *dev_cgroup, /* * In two cases we'll consider this new exception valid: - * - the dev cgroup has its default policy to allow + exception list: - * the new exception should *not* match any of the exceptions - * (behavior == DEVCG_DEFAULT_ALLOW, !match) * - the dev cgroup has its default policy to deny + exception list: * the new exception *should* match the exceptions - * (behavior == DEVCG_DEFAULT_DENY, match) + * - the dev cgroup has its default policy to allow + exception list: + * the new exception should *not* match any of the exceptions */ - if ((dev_cgroup->behavior == DEVCG_DEFAULT_DENY) == match) - return 1; - return 0; + if (dev_cgroup->behavior == DEVCG_DEFAULT_DENY) { + if (match) + return true; + } else { + if (!match) + return true; + } + return false; } /* -- cgit v1.2.3 From c39a2a3018f8065cb5ea38b0314c1bbedb2cfa0d Mon Sep 17 00:00:00 2001 From: Aristeu Rozanski Date: Fri, 15 Feb 2013 11:55:45 -0500 Subject: devcg: prepare may_access() for hierarchy support Currently may_access() is only able to verify if an exception is valid for the current cgroup, which has the same behavior. With hierarchy, it'll be also used to verify if a cgroup local exception is valid towards its cgroup parent, which might have different behavior. v2: - updated patch description - rebased on top of a new patch to expand the may_access() logic to make it more clear - fixed argument description order in may_access() Acked-by: Tejun Heo Acked-by: Serge Hallyn Cc: Tejun Heo Cc: Serge Hallyn Signed-off-by: Aristeu Rozanski Signed-off-by: Tejun Heo --- security/device_cgroup.c | 49 ++++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 18 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 4ad55a9c692..ddf3c709c0c 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -25,6 +25,12 @@ static DEFINE_MUTEX(devcgroup_mutex); +enum devcg_behavior { + DEVCG_DEFAULT_NONE, + DEVCG_DEFAULT_ALLOW, + DEVCG_DEFAULT_DENY, +}; + /* * exception list locking rules: * hold devcgroup_mutex for update/read. @@ -42,10 +48,7 @@ struct dev_exception_item { struct dev_cgroup { struct cgroup_subsys_state css; struct list_head exceptions; - enum { - DEVCG_DEFAULT_ALLOW, - DEVCG_DEFAULT_DENY, - } behavior; + enum devcg_behavior behavior; }; static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s) @@ -304,9 +307,11 @@ static int devcgroup_seq_read(struct cgroup *cgroup, struct cftype *cft, * verify if a certain access is allowed. * @dev_cgroup: dev cgroup to be tested against * @refex: new exception + * @behavior: behavior of the exception */ static bool may_access(struct dev_cgroup *dev_cgroup, - struct dev_exception_item *refex) + struct dev_exception_item *refex, + enum devcg_behavior behavior) { struct dev_exception_item *ex; bool match = false; @@ -330,19 +335,27 @@ static bool may_access(struct dev_cgroup *dev_cgroup, break; } - /* - * In two cases we'll consider this new exception valid: - * - the dev cgroup has its default policy to deny + exception list: - * the new exception *should* match the exceptions - * - the dev cgroup has its default policy to allow + exception list: - * the new exception should *not* match any of the exceptions - */ - if (dev_cgroup->behavior == DEVCG_DEFAULT_DENY) { - if (match) + if (dev_cgroup->behavior == DEVCG_DEFAULT_ALLOW) { + if (behavior == DEVCG_DEFAULT_ALLOW) { + /* the exception will deny access to certain devices */ + return true; + } else { + /* the exception will allow access to certain devices */ + if (match) + /* + * a new exception allowing access shouldn't + * match an parent's exception + */ + return false; return true; + } } else { - if (!match) + /* only behavior == DEVCG_DEFAULT_DENY allowed here */ + if (match) + /* parent has an exception that matches the proposed */ return true; + else + return false; } return false; } @@ -361,7 +374,7 @@ static int parent_has_perm(struct dev_cgroup *childcg, if (!pcg) return 1; parent = cgroup_to_devcgroup(pcg); - return may_access(parent, ex); + return may_access(parent, ex, childcg->behavior); } /** @@ -395,7 +408,7 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, { const char *b; char temp[12]; /* 11 + 1 characters needed for a u32 */ - int count, rc; + int count, rc = 0; struct dev_exception_item ex; struct cgroup *p = devcgroup->css.cgroup; struct dev_cgroup *parent = NULL; @@ -612,7 +625,7 @@ static int __devcgroup_check_permission(short type, u32 major, u32 minor, rcu_read_lock(); dev_cgroup = task_devcgroup(current); - rc = may_access(dev_cgroup, &ex); + rc = may_access(dev_cgroup, &ex, dev_cgroup->behavior); rcu_read_unlock(); if (!rc) -- cgit v1.2.3 From 1909554c9715e4d032497993bb56f2726bfa89ae Mon Sep 17 00:00:00 2001 From: Aristeu Rozanski Date: Fri, 15 Feb 2013 11:55:46 -0500 Subject: devcg: use css_online and css_offline Allocate resources and change behavior only when online. This is needed in order to determine if a node is suitable for hierarchy propagation or if it's being removed. Locking: Both functions take devcgroup_mutex to make changes to device_cgroup structure. Hierarchy propagation will also take devcgroup_mutex before walking the tree while walking the tree itself is protected by rcu lock. Acked-by: Tejun Heo Acked-by: Serge Hallyn Cc: Tejun Heo Cc: Serge Hallyn Signed-off-by: Aristeu Rozanski Signed-off-by: Tejun Heo --- security/device_cgroup.c | 59 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 17 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index ddf3c709c0c..16c9e1069be 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -185,36 +185,59 @@ static void dev_exception_clean(struct dev_cgroup *dev_cgroup) __dev_exception_clean(dev_cgroup); } +/** + * devcgroup_online - initializes devcgroup's behavior and exceptions based on + * parent's + * @cgroup: cgroup getting online + * returns 0 in case of success, error code otherwise + */ +static int devcgroup_online(struct cgroup *cgroup) +{ + struct dev_cgroup *dev_cgroup, *parent_dev_cgroup = NULL; + int ret = 0; + + mutex_lock(&devcgroup_mutex); + dev_cgroup = cgroup_to_devcgroup(cgroup); + if (cgroup->parent) + parent_dev_cgroup = cgroup_to_devcgroup(cgroup->parent); + + if (parent_dev_cgroup == NULL) + dev_cgroup->behavior = DEVCG_DEFAULT_ALLOW; + else { + ret = dev_exceptions_copy(&dev_cgroup->exceptions, + &parent_dev_cgroup->exceptions); + if (!ret) + dev_cgroup->behavior = parent_dev_cgroup->behavior; + } + mutex_unlock(&devcgroup_mutex); + + return ret; +} + +static void devcgroup_offline(struct cgroup *cgroup) +{ + struct dev_cgroup *dev_cgroup = cgroup_to_devcgroup(cgroup); + + mutex_lock(&devcgroup_mutex); + dev_cgroup->behavior = DEVCG_DEFAULT_NONE; + mutex_unlock(&devcgroup_mutex); +} + /* * called from kernel/cgroup.c with cgroup_lock() held. */ static struct cgroup_subsys_state *devcgroup_css_alloc(struct cgroup *cgroup) { - struct dev_cgroup *dev_cgroup, *parent_dev_cgroup; + struct dev_cgroup *dev_cgroup; struct cgroup *parent_cgroup; - int ret; dev_cgroup = kzalloc(sizeof(*dev_cgroup), GFP_KERNEL); if (!dev_cgroup) return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(&dev_cgroup->exceptions); + dev_cgroup->behavior = DEVCG_DEFAULT_NONE; parent_cgroup = cgroup->parent; - if (parent_cgroup == NULL) - dev_cgroup->behavior = DEVCG_DEFAULT_ALLOW; - else { - parent_dev_cgroup = cgroup_to_devcgroup(parent_cgroup); - mutex_lock(&devcgroup_mutex); - ret = dev_exceptions_copy(&dev_cgroup->exceptions, - &parent_dev_cgroup->exceptions); - dev_cgroup->behavior = parent_dev_cgroup->behavior; - mutex_unlock(&devcgroup_mutex); - if (ret) { - kfree(dev_cgroup); - return ERR_PTR(ret); - } - } - return &dev_cgroup->css; } @@ -587,6 +610,8 @@ struct cgroup_subsys devices_subsys = { .can_attach = devcgroup_can_attach, .css_alloc = devcgroup_css_alloc, .css_free = devcgroup_css_free, + .css_online = devcgroup_online, + .css_offline = devcgroup_offline, .subsys_id = devices_subsys_id, .base_cftypes = dev_cgroup_files, -- cgit v1.2.3 From bd2953ebbb533aeda9b86c82a53d5197a9a38f1b Mon Sep 17 00:00:00 2001 From: Aristeu Rozanski Date: Fri, 15 Feb 2013 11:55:47 -0500 Subject: devcg: propagate local changes down the hierarchy This patch makes exception changes to propagate down in hierarchy respecting when possible local exceptions. New exceptions allowing additional access to devices won't be propagated, but it'll be possible to add an exception to access all of part of the newly allowed device(s). New exceptions disallowing access to devices will be propagated down and the local group's exceptions will be revalidated for the new situation. Example: A / \ B group behavior exceptions A allow "b 8:* rwm", "c 116:1 rw" B deny "c 1:3 rwm", "c 116:2 rwm", "b 3:* rwm" If a new exception is added to group A: # echo "c 116:* r" > A/devices.deny it'll propagate down and after revalidating B's local exceptions, the exception "c 116:2 rwm" will be removed. In case parent's exceptions change and local exceptions are not allowed anymore, they'll be deleted. v7: - do not allow behavior change when the cgroup has children - update documentation v6: fixed issues pointed by Serge Hallyn - only copy parent's exceptions while propagating behavior if the local behavior is different - while propagating exceptions, do not clear and copy parent's: it'd be against the premise we don't propagate access to more devices v5: fixed issues pointed by Serge Hallyn - updated documentation - not propagating when an exception is written to devices.allow - when propagating a new behavior, clean the local exceptions list if they're for a different behavior v4: fixed issues pointed by Tejun Heo - separated function to walk the tree and collect valid propagation targets v3: fixed issues pointed by Tejun Heo - update documentation - move css_online/css_offline changes to a new patch - use cgroup_for_each_descendant_pre() instead of own descendant walk - move exception_copy rework to a separared patch - move exception_clean rework to a separated patch v2: fixed issues pointed by Tejun Heo - instead of keeping the local settings that won't apply anymore, remove them Cc: Tejun Heo Cc: Serge Hallyn Signed-off-by: Aristeu Rozanski Signed-off-by: Tejun Heo --- security/device_cgroup.c | 139 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 132 insertions(+), 7 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 16c9e1069be..221967d4690 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -49,6 +49,8 @@ struct dev_cgroup { struct cgroup_subsys_state css; struct list_head exceptions; enum devcg_behavior behavior; + /* temporary list for pending propagation operations */ + struct list_head propagate_pending; }; static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s) @@ -185,6 +187,11 @@ static void dev_exception_clean(struct dev_cgroup *dev_cgroup) __dev_exception_clean(dev_cgroup); } +static inline bool is_devcg_online(const struct dev_cgroup *devcg) +{ + return (devcg->behavior != DEVCG_DEFAULT_NONE); +} + /** * devcgroup_online - initializes devcgroup's behavior and exceptions based on * parent's @@ -235,6 +242,7 @@ static struct cgroup_subsys_state *devcgroup_css_alloc(struct cgroup *cgroup) if (!dev_cgroup) return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(&dev_cgroup->exceptions); + INIT_LIST_HEAD(&dev_cgroup->propagate_pending); dev_cgroup->behavior = DEVCG_DEFAULT_NONE; parent_cgroup = cgroup->parent; @@ -413,6 +421,111 @@ static inline int may_allow_all(struct dev_cgroup *parent) return parent->behavior == DEVCG_DEFAULT_ALLOW; } +/** + * revalidate_active_exceptions - walks through the active exception list and + * revalidates the exceptions based on parent's + * behavior and exceptions. The exceptions that + * are no longer valid will be removed. + * Called with devcgroup_mutex held. + * @devcg: cgroup which exceptions will be checked + * + * This is one of the three key functions for hierarchy implementation. + * This function is responsible for re-evaluating all the cgroup's active + * exceptions due to a parent's exception change. + * Refer to Documentation/cgroups/devices.txt for more details. + */ +static void revalidate_active_exceptions(struct dev_cgroup *devcg) +{ + struct dev_exception_item *ex; + struct list_head *this, *tmp; + + list_for_each_safe(this, tmp, &devcg->exceptions) { + ex = container_of(this, struct dev_exception_item, list); + if (!parent_has_perm(devcg, ex)) + dev_exception_rm(devcg, ex); + } +} + +/** + * get_online_devcg - walks the cgroup tree and fills a list with the online + * groups + * @root: cgroup used as starting point + * @online: list that will be filled with online groups + * + * Must be called with devcgroup_mutex held. Grabs RCU lock. + * Because devcgroup_mutex is held, no devcg will become online or offline + * during the tree walk (see devcgroup_online, devcgroup_offline) + * A separated list is needed because propagate_behavior() and + * propagate_exception() need to allocate memory and can block. + */ +static void get_online_devcg(struct cgroup *root, struct list_head *online) +{ + struct cgroup *pos; + struct dev_cgroup *devcg; + + lockdep_assert_held(&devcgroup_mutex); + + rcu_read_lock(); + cgroup_for_each_descendant_pre(pos, root) { + devcg = cgroup_to_devcgroup(pos); + if (is_devcg_online(devcg)) + list_add_tail(&devcg->propagate_pending, online); + } + rcu_read_unlock(); +} + +/** + * propagate_exception - propagates a new exception to the children + * @devcg_root: device cgroup that added a new exception + * @ex: new exception to be propagated + * + * returns: 0 in case of success, != 0 in case of error + */ +static int propagate_exception(struct dev_cgroup *devcg_root, + struct dev_exception_item *ex) +{ + struct cgroup *root = devcg_root->css.cgroup; + struct dev_cgroup *devcg, *parent, *tmp; + int rc = 0; + LIST_HEAD(pending); + + get_online_devcg(root, &pending); + + list_for_each_entry_safe(devcg, tmp, &pending, propagate_pending) { + parent = cgroup_to_devcgroup(devcg->css.cgroup->parent); + + /* + * in case both root's behavior and devcg is allow, a new + * restriction means adding to the exception list + */ + if (devcg_root->behavior == DEVCG_DEFAULT_ALLOW && + devcg->behavior == DEVCG_DEFAULT_ALLOW) { + rc = dev_exception_add(devcg, ex); + if (rc) + break; + } else { + /* + * in the other possible cases: + * root's behavior: allow, devcg's: deny + * root's behavior: deny, devcg's: deny + * the exception will be removed + */ + dev_exception_rm(devcg, ex); + } + revalidate_active_exceptions(devcg); + + list_del_init(&devcg->propagate_pending); + } + return rc; +} + +static inline bool has_children(struct dev_cgroup *devcgroup) +{ + struct cgroup *cgrp = devcgroup->css.cgroup; + + return !list_empty(&cgrp->children); +} + /* * Modify the exception list using allow/deny rules. * CAP_SYS_ADMIN is needed for this. It's at least separate from CAP_MKNOD @@ -449,6 +562,9 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, case 'a': switch (filetype) { case DEVCG_ALLOW: + if (has_children(devcgroup)) + return -EINVAL; + if (!may_allow_all(parent)) return -EPERM; dev_exception_clean(devcgroup); @@ -462,6 +578,9 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, return rc; break; case DEVCG_DENY: + if (has_children(devcgroup)) + return -EINVAL; + dev_exception_clean(devcgroup); devcgroup->behavior = DEVCG_DEFAULT_DENY; break; @@ -556,22 +675,28 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup, dev_exception_rm(devcgroup, &ex); return 0; } - return dev_exception_add(devcgroup, &ex); + rc = dev_exception_add(devcgroup, &ex); + break; case DEVCG_DENY: /* * If the default policy is to deny by default, try to remove * an matching exception instead. And be silent about it: we * don't want to break compatibility */ - if (devcgroup->behavior == DEVCG_DEFAULT_DENY) { + if (devcgroup->behavior == DEVCG_DEFAULT_DENY) dev_exception_rm(devcgroup, &ex); - return 0; - } - return dev_exception_add(devcgroup, &ex); + else + rc = dev_exception_add(devcgroup, &ex); + + if (rc) + break; + /* we only propagate new restrictions */ + rc = propagate_exception(devcgroup, &ex); + break; default: - return -EINVAL; + rc = -EINVAL; } - return 0; + return rc; } static int devcgroup_access_write(struct cgroup *cgrp, struct cftype *cft, -- cgit v1.2.3 From 8adf12b0ffd0a10340bab355e586f8533a69630d Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Sun, 7 Apr 2013 10:25:39 -0700 Subject: devcg: remove broken_hierarchy tag bd2953ebbb ("devcg: propagate local changes down the hierarchy") implemented proper hierarchy support. Remove the broken tag. Signed-off-by: Tejun Heo Acked-by: Aristeu Rozanski --- security/device_cgroup.c | 9 --------- 1 file changed, 9 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 221967d4690..994aeccea51 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -739,15 +739,6 @@ struct cgroup_subsys devices_subsys = { .css_offline = devcgroup_offline, .subsys_id = devices_subsys_id, .base_cftypes = dev_cgroup_files, - - /* - * While devices cgroup has the rudimentary hierarchy support which - * checks the parent's restriction, it doesn't properly propagates - * config changes in ancestors to their descendents. A child - * should only be allowed to add more restrictions to the parent's - * configuration. Fix it and remove the following. - */ - .broken_hierarchy = true, }; /** -- cgit v1.2.3 From e57d5cf2f894e3f2727f8cf74bed0bc81cae70c8 Mon Sep 17 00:00:00 2001 From: Rami Rosen Date: Tue, 16 Apr 2013 23:24:55 +0300 Subject: devcg: remove parent_cgroup. In devcgroup_css_alloc(), there is no longer need for parent_cgroup. bd2953ebbb("devcg: propagate local changes down the hierarchy") made the variable parent_cgroup redundant. This patch removes parent_cgroup from devcgroup_css_alloc(). Signed-off-by: Rami Rosen Acked-by: Aristeu Rozanski Signed-off-by: Tejun Heo --- security/device_cgroup.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'security') diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 994aeccea51..dd0dc574d78 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -236,7 +236,6 @@ static void devcgroup_offline(struct cgroup *cgroup) static struct cgroup_subsys_state *devcgroup_css_alloc(struct cgroup *cgroup) { struct dev_cgroup *dev_cgroup; - struct cgroup *parent_cgroup; dev_cgroup = kzalloc(sizeof(*dev_cgroup), GFP_KERNEL); if (!dev_cgroup) @@ -244,7 +243,6 @@ static struct cgroup_subsys_state *devcgroup_css_alloc(struct cgroup *cgroup) INIT_LIST_HEAD(&dev_cgroup->exceptions); INIT_LIST_HEAD(&dev_cgroup->propagate_pending); dev_cgroup->behavior = DEVCG_DEFAULT_NONE; - parent_cgroup = cgroup->parent; return &dev_cgroup->css; } -- cgit v1.2.3