Re: [PATCH v2 5/6] device_cgroup: rename whitelist to exception list

["Serge E. Hallyn" <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org>]

Quoting Aristeu Rozanski (aris-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
> 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 <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Li Zefan <lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
> Cc: Pavel Emelyanov <xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
> Cc: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

> Signed-off-by: Aristeu Rozanski <aris-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 
> ---
>  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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/