From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758095Ab2IEDX3 (ORCPT ); Tue, 4 Sep 2012 23:23:29 -0400 Received: from 50-56-35-84.static.cloud-ips.com ([50.56.35.84]:36276 "EHLO mail.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751768Ab2IEDX2 (ORCPT ); Tue, 4 Sep 2012 23:23:28 -0400 Date: Wed, 5 Sep 2012 03:24:02 +0000 From: "Serge E. Hallyn" To: Aristeu Rozanski Cc: linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, Tejun Heo , Li Zefan , James Morris , Pavel Emelyanov , Serge Hallyn , Andrew Morton Subject: Re: [PATCH v2 5/6] device_cgroup: rename whitelist to exception list Message-ID: <20120905032402.GE13310@mail.hallyn.com> References: <20120904143419.892872876@napanee.usersys.redhat.com> <20120904143421.423387352@napanee.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120904143421.423387352@napanee.usersys.redhat.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Quoting Aristeu Rozanski (aris@redhat.com): > This patch replaces the "whitelist" usage in the code and comments and replace > them by exception list related information. > > v2: > - fix checkpatch warnings > > Cc: Tejun Heo > Cc: Li Zefan > Cc: James Morris > Cc: Pavel Emelyanov > Cc: Serge Hallyn Acked-by: Serge Hallyn > Signed-off-by: Aristeu Rozanski > > --- > security/device_cgroup.c | 198 +++++++++++++++++++++++------------------------ > 1 file changed, 99 insertions(+), 99 deletions(-) > > Index: github/security/device_cgroup.c > =================================================================== > --- github.orig/security/device_cgroup.c 2012-08-21 10:51:15.000000000 -0400 > +++ github/security/device_cgroup.c 2012-08-21 10:54:16.509079085 -0400 > @@ -26,12 +26,12 @@ > static DEFINE_MUTEX(devcgroup_mutex); > > /* > - * whitelist locking rules: > + * exception list locking rules: > * hold devcgroup_mutex for update/read. > * hold rcu_read_lock() for read. > */ > > -struct dev_whitelist_item { > +struct dev_exception_item { > u32 major, minor; > short type; > short access; > @@ -41,7 +41,7 @@ > > struct dev_cgroup { > struct cgroup_subsys_state css; > - struct list_head whitelist; > + struct list_head exceptions; > enum { > DEVCG_DEFAULT_ALLOW, > DEVCG_DEFAULT_DENY, > @@ -78,12 +78,12 @@ > /* > * called under devcgroup_mutex > */ > -static int dev_whitelist_copy(struct list_head *dest, struct list_head *orig) > +static int dev_exceptions_copy(struct list_head *dest, struct list_head *orig) > { > - struct dev_whitelist_item *wh, *tmp, *new; > + struct dev_exception_item *ex, *tmp, *new; > > - list_for_each_entry(wh, orig, list) { > - new = kmemdup(wh, sizeof(*wh), GFP_KERNEL); > + list_for_each_entry(ex, orig, list) { > + new = kmemdup(ex, sizeof(*ex), GFP_KERNEL); > if (!new) > goto free_and_exit; > list_add_tail(&new->list, dest); > @@ -92,9 +92,9 @@ > return 0; > > free_and_exit: > - list_for_each_entry_safe(wh, tmp, dest, list) { > - list_del(&wh->list); > - kfree(wh); > + list_for_each_entry_safe(ex, tmp, dest, list) { > + list_del(&ex->list); > + kfree(ex); > } > return -ENOMEM; > } > @@ -102,50 +102,50 @@ > /* > * called under devcgroup_mutex > */ > -static int dev_whitelist_add(struct dev_cgroup *dev_cgroup, > - struct dev_whitelist_item *wh) > +static int dev_exception_add(struct dev_cgroup *dev_cgroup, > + struct dev_exception_item *ex) > { > - struct dev_whitelist_item *whcopy, *walk; > + struct dev_exception_item *excopy, *walk; > > - whcopy = kmemdup(wh, sizeof(*wh), GFP_KERNEL); > - if (!whcopy) > + excopy = kmemdup(ex, sizeof(*ex), GFP_KERNEL); > + if (!excopy) > return -ENOMEM; > > - list_for_each_entry(walk, &dev_cgroup->whitelist, list) { > - if (walk->type != wh->type) > + list_for_each_entry(walk, &dev_cgroup->exceptions, list) { > + if (walk->type != ex->type) > continue; > - if (walk->major != wh->major) > + if (walk->major != ex->major) > continue; > - if (walk->minor != wh->minor) > + if (walk->minor != ex->minor) > continue; > > - walk->access |= wh->access; > - kfree(whcopy); > - whcopy = NULL; > + walk->access |= ex->access; > + kfree(excopy); > + excopy = NULL; > } > > - if (whcopy != NULL) > - list_add_tail_rcu(&whcopy->list, &dev_cgroup->whitelist); > + if (excopy != NULL) > + list_add_tail_rcu(&excopy->list, &dev_cgroup->exceptions); > return 0; > } > > /* > * called under devcgroup_mutex > */ > -static void dev_whitelist_rm(struct dev_cgroup *dev_cgroup, > - struct dev_whitelist_item *wh) > +static void dev_exception_rm(struct dev_cgroup *dev_cgroup, > + struct dev_exception_item *ex) > { > - struct dev_whitelist_item *walk, *tmp; > + struct dev_exception_item *walk, *tmp; > > - list_for_each_entry_safe(walk, tmp, &dev_cgroup->whitelist, list) { > - if (walk->type != wh->type) > + list_for_each_entry_safe(walk, tmp, &dev_cgroup->exceptions, list) { > + if (walk->type != ex->type) > continue; > - if (walk->major != wh->major) > + if (walk->major != ex->major) > continue; > - if (walk->minor != wh->minor) > + if (walk->minor != ex->minor) > continue; > > - walk->access &= ~wh->access; > + walk->access &= ~ex->access; > if (!walk->access) { > list_del_rcu(&walk->list); > kfree_rcu(walk, rcu); > @@ -154,18 +154,18 @@ > } > > /** > - * dev_whitelist_clean - frees all entries of the whitelist > - * @dev_cgroup: dev_cgroup with the whitelist to be cleaned > + * dev_exception_clean - frees all entries of the exception list > + * @dev_cgroup: dev_cgroup with the exception list to be cleaned > * > * called under devcgroup_mutex > */ > -static void dev_whitelist_clean(struct dev_cgroup *dev_cgroup) > +static void dev_exception_clean(struct dev_cgroup *dev_cgroup) > { > - struct dev_whitelist_item *wh, *tmp; > + struct dev_exception_item *ex, *tmp; > > - list_for_each_entry_safe(wh, tmp, &dev_cgroup->whitelist, list) { > - list_del(&wh->list); > - kfree(wh); > + list_for_each_entry_safe(ex, tmp, &dev_cgroup->exceptions, list) { > + list_del(&ex->list); > + kfree(ex); > } > } > > @@ -181,7 +181,7 @@ > dev_cgroup = kzalloc(sizeof(*dev_cgroup), GFP_KERNEL); > if (!dev_cgroup) > return ERR_PTR(-ENOMEM); > - INIT_LIST_HEAD(&dev_cgroup->whitelist); > + INIT_LIST_HEAD(&dev_cgroup->exceptions); > parent_cgroup = cgroup->parent; > > if (parent_cgroup == NULL) > @@ -189,8 +189,8 @@ > else { > parent_dev_cgroup = cgroup_to_devcgroup(parent_cgroup); > mutex_lock(&devcgroup_mutex); > - ret = dev_whitelist_copy(&dev_cgroup->whitelist, > - &parent_dev_cgroup->whitelist); > + ret = dev_exceptions_copy(&dev_cgroup->exceptions, > + &parent_dev_cgroup->exceptions); > dev_cgroup->behavior = parent_dev_cgroup->behavior; > mutex_unlock(&devcgroup_mutex); > if (ret) { > @@ -207,7 +207,7 @@ > struct dev_cgroup *dev_cgroup; > > dev_cgroup = cgroup_to_devcgroup(cgroup); > - dev_whitelist_clean(dev_cgroup); > + dev_exception_clean(dev_cgroup); > kfree(dev_cgroup); > } > > @@ -253,7 +253,7 @@ > struct seq_file *m) > { > struct dev_cgroup *devcgroup = cgroup_to_devcgroup(cgroup); > - struct dev_whitelist_item *wh; > + struct dev_exception_item *ex; > char maj[MAJMINLEN], min[MAJMINLEN], acc[ACCLEN]; > > rcu_read_lock(); > @@ -270,11 +270,11 @@ > seq_printf(m, "%c %s:%s %s\n", type_to_char(DEV_ALL), > maj, min, acc); > } else { > - list_for_each_entry_rcu(wh, &devcgroup->whitelist, list) { > - set_access(acc, wh->access); > - set_majmin(maj, wh->major); > - set_majmin(min, wh->minor); > - seq_printf(m, "%c %s:%s %s\n", type_to_char(wh->type), > + list_for_each_entry_rcu(ex, &devcgroup->exceptions, list) { > + set_access(acc, ex->access); > + set_majmin(maj, ex->major); > + set_majmin(min, ex->minor); > + seq_printf(m, "%c %s:%s %s\n", type_to_char(ex->type), > maj, min, acc); > } > } > @@ -284,42 +284,42 @@ > } > > /** > - * may_access_whitelist - verifies if a new rule is part of what is allowed > - * by a dev cgroup based on the default policy + > - * exceptions. This is used to make sure a child cgroup > - * won't have more privileges than its parent or to > - * verify if a certain access is allowed. > + * may_access - verifies if a new exception is part of what is allowed > + * by a dev cgroup based on the default policy + > + * exceptions. This is used to make sure a child cgroup > + * won't have more privileges than its parent or to > + * verify if a certain access is allowed. > * @dev_cgroup: dev cgroup to be tested against > - * @refwh: new rule > + * @refex: new exception > */ > -static int may_access_whitelist(struct dev_cgroup *dev_cgroup, > - struct dev_whitelist_item *refwh) > +static int may_access(struct dev_cgroup *dev_cgroup, > + struct dev_exception_item *refex) > { > - struct dev_whitelist_item *whitem; > + struct dev_exception_item *ex; > bool match = false; > > - list_for_each_entry(whitem, &dev_cgroup->whitelist, list) { > - if ((refwh->type & DEV_BLOCK) && !(whitem->type & DEV_BLOCK)) > + list_for_each_entry(ex, &dev_cgroup->exceptions, list) { > + if ((refex->type & DEV_BLOCK) && !(ex->type & DEV_BLOCK)) > continue; > - if ((refwh->type & DEV_CHAR) && !(whitem->type & DEV_CHAR)) > + if ((refex->type & DEV_CHAR) && !(ex->type & DEV_CHAR)) > continue; > - if (whitem->major != ~0 && whitem->major != refwh->major) > + if (ex->major != ~0 && ex->major != refex->major) > continue; > - if (whitem->minor != ~0 && whitem->minor != refwh->minor) > + if (ex->minor != ~0 && ex->minor != refex->minor) > continue; > - if (refwh->access & (~whitem->access)) > + if (refex->access & (~ex->access)) > continue; > match = true; > break; > } > > /* > - * In two cases we'll consider this new rule valid: > + * In two cases we'll consider this new exception valid: > * - the dev cgroup has its default policy to allow + exception list: > - * the new rule should *not* match any of the exceptions > + * the new exception should *not* match any of the exceptions > * (behavior != DEVCG_DEFAULT_DENY, !match) > * - the dev cgroup has its default policy to deny + exception list: > - * the new rule *should* match the exceptions > + * the new exception *should* match the exceptions > * (behavior == DEVCG_DEFAULT_DENY, match) > */ > if ((dev_cgroup->behavior == DEVCG_DEFAULT_DENY) == match) > @@ -329,11 +329,11 @@ > > /* > * parent_has_perm: > - * when adding a new allow rule to a device whitelist, the rule > + * when adding a new allow rule to a device exception list, the rule > * must be allowed in the parent device > */ > static int parent_has_perm(struct dev_cgroup *childcg, > - struct dev_whitelist_item *wh) > + struct dev_exception_item *ex) > { > struct cgroup *pcg = childcg->css.cgroup->parent; > struct dev_cgroup *parent; > @@ -341,17 +341,17 @@ > if (!pcg) > return 1; > parent = cgroup_to_devcgroup(pcg); > - return may_access_whitelist(parent, wh); > + return may_access(parent, ex); > } > > /* > - * Modify the whitelist using allow/deny rules. > + * Modify the exception list using allow/deny rules. > * CAP_SYS_ADMIN is needed for this. It's at least separate from CAP_MKNOD > * so we can give a container CAP_MKNOD to let it create devices but not > - * modify the whitelist. > + * modify the exception list. > * It seems likely we'll want to add a CAP_CONTAINER capability to allow > * us to also grant CAP_SYS_ADMIN to containers without giving away the > - * device whitelist controls, but for now we'll stick with CAP_SYS_ADMIN > + * device exception list controls, but for now we'll stick with CAP_SYS_ADMIN > * > * Taking rules away is always allowed (given CAP_SYS_ADMIN). Granting > * new access is only allowed if you're in the top-level cgroup, or your > @@ -363,25 +363,25 @@ > const char *b; > char temp[12]; /* 11 + 1 characters needed for a u32 */ > int count, rc; > - struct dev_whitelist_item wh; > + struct dev_exception_item ex; > > if (!capable(CAP_SYS_ADMIN)) > return -EPERM; > > - memset(&wh, 0, sizeof(wh)); > + memset(&ex, 0, sizeof(ex)); > b = buffer; > > switch (*b) { > case 'a': > switch (filetype) { > case DEVCG_ALLOW: > - if (!parent_has_perm(devcgroup, &wh)) > + if (!parent_has_perm(devcgroup, &ex)) > return -EPERM; > - dev_whitelist_clean(devcgroup); > + dev_exception_clean(devcgroup); > devcgroup->behavior = DEVCG_DEFAULT_ALLOW; > break; > case DEVCG_DENY: > - dev_whitelist_clean(devcgroup); > + dev_exception_clean(devcgroup); > devcgroup->behavior = DEVCG_DEFAULT_DENY; > break; > default: > @@ -389,10 +389,10 @@ > } > return 0; > case 'b': > - wh.type = DEV_BLOCK; > + ex.type = DEV_BLOCK; > break; > case 'c': > - wh.type = DEV_CHAR; > + ex.type = DEV_CHAR; > break; > default: > return -EINVAL; > @@ -402,7 +402,7 @@ > return -EINVAL; > b++; > if (*b == '*') { > - wh.major = ~0; > + ex.major = ~0; > b++; > } else if (isdigit(*b)) { > memset(temp, 0, sizeof(temp)); > @@ -412,7 +412,7 @@ > if (!isdigit(*b)) > break; > } > - rc = kstrtou32(temp, 10, &wh.major); > + rc = kstrtou32(temp, 10, &ex.major); > if (rc) > return -EINVAL; > } else { > @@ -424,7 +424,7 @@ > > /* read minor */ > if (*b == '*') { > - wh.minor = ~0; > + ex.minor = ~0; > b++; > } else if (isdigit(*b)) { > memset(temp, 0, sizeof(temp)); > @@ -434,7 +434,7 @@ > if (!isdigit(*b)) > break; > } > - rc = kstrtou32(temp, 10, &wh.minor); > + rc = kstrtou32(temp, 10, &ex.minor); > if (rc) > return -EINVAL; > } else { > @@ -445,13 +445,13 @@ > for (b++, count = 0; count < 3; count++, b++) { > switch (*b) { > case 'r': > - wh.access |= ACC_READ; > + ex.access |= ACC_READ; > break; > case 'w': > - wh.access |= ACC_WRITE; > + ex.access |= ACC_WRITE; > break; > case 'm': > - wh.access |= ACC_MKNOD; > + ex.access |= ACC_MKNOD; > break; > case '\n': > case '\0': > @@ -464,7 +464,7 @@ > > switch (filetype) { > case DEVCG_ALLOW: > - if (!parent_has_perm(devcgroup, &wh)) > + if (!parent_has_perm(devcgroup, &ex)) > return -EPERM; > /* > * If the default policy is to allow by default, try to remove > @@ -472,10 +472,10 @@ > * don't want to break compatibility > */ > if (devcgroup->behavior == DEVCG_DEFAULT_ALLOW) { > - dev_whitelist_rm(devcgroup, &wh); > + dev_exception_rm(devcgroup, &ex); > return 0; > } > - return dev_whitelist_add(devcgroup, &wh); > + return dev_exception_add(devcgroup, &ex); > case DEVCG_DENY: > /* > * If the default policy is to deny by default, try to remove > @@ -483,10 +483,10 @@ > * don't want to break compatibility > */ > if (devcgroup->behavior == DEVCG_DEFAULT_DENY) { > - dev_whitelist_rm(devcgroup, &wh); > + dev_exception_rm(devcgroup, &ex); > return 0; > } > - return dev_whitelist_add(devcgroup, &wh); > + return dev_exception_add(devcgroup, &ex); > default: > return -EINVAL; > } > @@ -547,17 +547,17 @@ > short type, u32 major, u32 minor, > short access) > { > - struct dev_whitelist_item wh; > + struct dev_exception_item ex; > int rc; > > - memset(&wh, 0, sizeof(wh)); > - wh.type = type; > - wh.major = major; > - wh.minor = minor; > - wh.access = access; > + memset(&ex, 0, sizeof(ex)); > + ex.type = type; > + ex.major = major; > + ex.minor = minor; > + ex.access = access; > > rcu_read_lock(); > - rc = may_access_whitelist(dev_cgroup, &wh); > + rc = may_access(dev_cgroup, &ex); > rcu_read_unlock(); > > if (!rc) > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/