Re: [PATCH v4 3/9] device_cgroup: keep track of local groupsettings

[Serge E. Hallyn]

Quoting aris@xxxxxxxxxx (aris@xxxxxxxxxx):
> In preparation for better hierarchy support, it's needed to retain the local
> settings in order to try to reapply them after a propagated change if they're
> still valid.
> 
> v2: split this patch in two, one to just move dev_exception_rm() before
>     dev_exception_add() while keeping functional changes in this patch as
>     requested by Tejun.
> 
> Acked-by: Tejun Heo <tj@xxxxxxxxxx>
> Cc: Tejun Heo <tj@xxxxxxxxxx>
> Cc: Serge Hallyn <serge.hallyn@xxxxxxxxxxxxx>

Acked-by: Serge Hallyn <serge.hallyn@xxxxxxxxxxxxx>

> Signed-off-by: Aristeu Rozanski <aris@xxxxxxxxxx>
>  
> ---
>  security/device_cgroup.c |   83 +++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 67 insertions(+), 16 deletions(-)
> 
> --- github.orig/security/device_cgroup.c	2013-01-29 11:49:14.987661210 -0500
> +++ github/security/device_cgroup.c	2013-01-29 11:49:15.244665037 -0500
> @@ -39,13 +39,27 @@ struct dev_exception_item {
>  	struct rcu_head rcu;
>  };
>  
> +enum devcg_behavior {
> +	DEVCG_DEFAULT_NONE,
> +	DEVCG_DEFAULT_ALLOW,
> +	DEVCG_DEFAULT_DENY,
> +};
> +
>  struct dev_cgroup {
>  	struct cgroup_subsys_state css;
> +
> +	/* result of merging the parent's rules with local ones */
>  	struct list_head exceptions;
> -	enum {
> -		DEVCG_DEFAULT_ALLOW,
> -		DEVCG_DEFAULT_DENY,
> -	} behavior;
> +	enum devcg_behavior behavior;
> +
> +	/*
> +	 * local set rules, saved so when a parent propagates new rules, the
> +	 * local preferences can be preserved
> +	 */
> +	struct {
> +		struct list_head exceptions;
> +		enum devcg_behavior behavior;
> +	} local;
>  };
>  
>  static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s)
> @@ -104,8 +118,8 @@ free_and_exit:
>  /*
>   * called under devcgroup_mutex
>   */
> -static void dev_exception_rm(struct list_head *exceptions,
> -			     struct dev_exception_item *ex)
> +static void __dev_exception_rm(struct list_head *exceptions,
> +			       struct dev_exception_item *ex)
>  {
>  	struct dev_exception_item *walk, *tmp;
>  
> @@ -127,11 +141,18 @@ static void dev_exception_rm(struct list
>  	}
>  }
>  
> +static void dev_exception_rm(struct dev_cgroup *devcgroup,
> +			     struct dev_exception_item *ex)
> +{
> +	__dev_exception_rm(&devcgroup->local.exceptions, ex);
> +	__dev_exception_rm(&devcgroup->exceptions, ex);
> +}
> +
>  /*
>   * called under devcgroup_mutex
>   */
> -static int dev_exception_add(struct list_head *exceptions,
> -			     struct dev_exception_item *ex)
> +static int __dev_exception_add(struct list_head *exceptions,
> +			       struct dev_exception_item *ex)
>  {
>  	struct dev_exception_item *excopy, *walk;
>  
> @@ -159,6 +180,28 @@ static int dev_exception_add(struct list
>  	return 0;
>  }
>  
> +static int dev_exception_add(struct dev_cgroup *devcgroup,
> +			     struct dev_exception_item *ex)
> +{
> +	int rc;
> +
> +	lockdep_assert_held(&devcgroup_mutex);
> +
> +	/*
> +	 * we add to the local list so we can preserve local preferences if
> +	 * the parent propagates down new rules
> +	 */
> +	rc = __dev_exception_add(&devcgroup->local.exceptions, ex);
> +	if (rc)
> +		return rc;
> +
> +	rc = __dev_exception_add(&devcgroup->exceptions, ex);
> +	if (rc)
> +		__dev_exception_rm(&devcgroup->local.exceptions, ex);
> +
> +	return rc;
> +}
> +
>  static void __dev_exception_clean(struct dev_cgroup *dev_cgroup)
>  {
>  	struct dev_exception_item *ex, *tmp;
> @@ -167,6 +210,11 @@ static void __dev_exception_clean(struct
>  		list_del_rcu(&ex->list);
>  		kfree_rcu(ex, rcu);
>  	}
> +	list_for_each_entry_safe(ex, tmp, &dev_cgroup->local.exceptions,
> +				 list) {
> +		list_del_rcu(&ex->list);
> +		kfree_rcu(ex, rcu);
> +	}
>  }
>  
>  /**
> @@ -195,6 +243,8 @@ static struct cgroup_subsys_state *devcg
>  	if (!dev_cgroup)
>  		return ERR_PTR(-ENOMEM);
>  	INIT_LIST_HEAD(&dev_cgroup->exceptions);
> +	INIT_LIST_HEAD(&dev_cgroup->local.exceptions);
> +	dev_cgroup->local.behavior = DEVCG_DEFAULT_NONE;
>  	parent_cgroup = cgroup->parent;
>  
>  	if (parent_cgroup == NULL)
> @@ -413,18 +463,19 @@ 	memset(&ex, 0, sizeof(ex));
>  			if (!may_allow_all(parent))
>  				return -EPERM;
>  			dev_exception_clean(devcgroup);
> +			if (parent)
> +				rc = dev_exceptions_copy(&devcgroup->exceptions,
> +							 &parent->exceptions);
>  			devcgroup->behavior = DEVCG_DEFAULT_ALLOW;
> -			if (!parent)
> -				break;
> +			devcgroup->local.behavior = DEVCG_DEFAULT_ALLOW;
>  
> -			rc = dev_exceptions_copy(&devcgroup->exceptions,
> -						 &parent->exceptions);
>  			if (rc)
>  				return rc;
>  			break;
>  		case DEVCG_DENY:
>  			dev_exception_clean(devcgroup);
>  			devcgroup->behavior = DEVCG_DEFAULT_DENY;
> +			devcgroup->local.behavior = DEVCG_DEFAULT_DENY;
>  			break;
>  		default:
>  			return -EINVAL;
> @@ -514,10 +565,10 @@ 		case '\0':
>  		 * don't want to break compatibility
>  		 */
>  		if (devcgroup->behavior == DEVCG_DEFAULT_ALLOW) {
> -			dev_exception_rm(&devcgroup->exceptions, &ex);
> +			dev_exception_rm(devcgroup, &ex);
>  			return 0;
>  		}
> -		return dev_exception_add(&devcgroup->exceptions, &ex);
> +		return dev_exception_add(devcgroup, &ex);
>  	case DEVCG_DENY:
>  		/*
>  		 * If the default policy is to deny by default, try to remove
> @@ -525,10 +576,10 @@ 			return 0;
>  		 * don't want to break compatibility
>  		 */
>  		if (devcgroup->behavior == DEVCG_DEFAULT_DENY) {
> -			dev_exception_rm(&devcgroup->exceptions, &ex);
> +			dev_exception_rm(devcgroup, &ex);
>  			return 0;
>  		}
> -		return dev_exception_add(&devcgroup->exceptions, &ex);
> +		return dev_exception_add(devcgroup, &ex);
>  	default:
>  		return -EINVAL;
>  	}
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/