2012-10-22 13:46:35

by Aristeu Rozanski

[permalink] [raw]
Subject: [PATCH 2/4] device_cgroup: rename deny_all to behavior

This was done in a v2 patch but v1 ended up being committed. The variable name
is less confusing and stores the default behavior when no matching exception
exists.


Cc: Dave Jones <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: James Morris <[email protected]>
Cc: Pavel Emelyanov <[email protected]>
Cc: Serge Hallyn <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Aristeu Rozanski <[email protected]>

---
security/device_cgroup.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)

--- github.orig/security/device_cgroup.c 2012-10-19 16:35:37.936804289 -0400
+++ github/security/device_cgroup.c 2012-10-19 16:35:46.366102913 -0400
@@ -42,7 +42,10 @@ struct dev_exception_item {
struct dev_cgroup {
struct cgroup_subsys_state css;
struct list_head exceptions;
- bool deny_all;
+ enum {
+ DEVCG_DEFAULT_ALLOW,
+ DEVCG_DEFAULT_DENY,
+ } behavior;
};

static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s)
@@ -182,13 +185,13 @@ static struct cgroup_subsys_state *devcg
parent_cgroup = cgroup->parent;

if (parent_cgroup == NULL)
- dev_cgroup->deny_all = false;
+ 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->deny_all = parent_dev_cgroup->deny_all;
+ dev_cgroup->behavior = parent_dev_cgroup->behavior;
mutex_unlock(&devcgroup_mutex);
if (ret) {
kfree(dev_cgroup);
@@ -260,7 +263,7 @@ static int devcgroup_seq_read(struct cgr
* - List the exceptions in case the default policy is to deny
* This way, the file remains as a "whitelist of devices"
*/
- if (devcgroup->deny_all == false) {
+ if (devcgroup->behavior == DEVCG_DEFAULT_ALLOW) {
set_access(acc, ACC_MASK);
set_majmin(maj, ~0);
set_majmin(min, ~0);
@@ -314,12 +317,12 @@ if (ex->minor != ~0 && ex->minor != re
* 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
- * (!deny_all, !match)
+ * (behavior == DEVCG_DEFAULT_ALLOW, !match)
* - the dev cgroup has its default policy to deny + exception list:
* the new exception *should* match the exceptions
- * (deny_all, match)
+ * (behavior == DEVCG_DEFAULT_DENY, match)
*/
- if (dev_cgroup->deny_all == match)
+ if ((dev_cgroup->behavior == DEVCG_DEFAULT_DENY) == match)
return 1;
return 0;
}
@@ -375,11 +378,11 @@ memset(&ex, 0, sizeof(ex));
if (!parent_has_perm(devcgroup, &ex))
return -EPERM;
dev_exception_clean(devcgroup);
- devcgroup->deny_all = false;
+ devcgroup->behavior = DEVCG_DEFAULT_ALLOW;
break;
case DEVCG_DENY:
dev_exception_clean(devcgroup);
- devcgroup->deny_all = true;
+ devcgroup->behavior = DEVCG_DEFAULT_DENY;
break;
default:
return -EINVAL;
@@ -452,7 +455,7 @@ case '\0':
* an matching exception instead. And be silent about it: we
* don't want to break compatibility
*/
- if (devcgroup->deny_all == false) {
+ if (devcgroup->behavior == DEVCG_DEFAULT_ALLOW) {
dev_exception_rm(devcgroup, &ex);
return 0;
}
@@ -463,7 +466,7 @@ return 0;
* an matching exception instead. And be silent about it: we
* don't want to break compatibility
*/
- if (devcgroup->deny_all == true) {
+ if (devcgroup->behavior == DEVCG_DEFAULT_DENY) {
dev_exception_rm(devcgroup, &ex);
return 0;
}


2012-10-22 16:13:05

by Serge Hallyn

[permalink] [raw]
Subject: Re: [PATCH 2/4] device_cgroup: rename deny_all to behavior

Quoting Aristeu Rozanski ([email protected]):
> This was done in a v2 patch but v1 ended up being committed. The variable name
> is less confusing and stores the default behavior when no matching exception
> exists.
>
>
> Cc: Dave Jones <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Tejun Heo <[email protected]>
> Cc: Li Zefan <[email protected]>
> Cc: James Morris <[email protected]>
> Cc: Pavel Emelyanov <[email protected]>
> Cc: Serge Hallyn <[email protected]>

Acked-by: Serge E. Hallyn <[email protected]>

> Cc: Jiri Slaby <[email protected]>
> Signed-off-by: Aristeu Rozanski <[email protected]>
>
> ---
> security/device_cgroup.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> --- github.orig/security/device_cgroup.c 2012-10-19 16:35:37.936804289 -0400
> +++ github/security/device_cgroup.c 2012-10-19 16:35:46.366102913 -0400
> @@ -42,7 +42,10 @@ struct dev_exception_item {
> struct dev_cgroup {
> struct cgroup_subsys_state css;
> struct list_head exceptions;
> - bool deny_all;
> + enum {
> + DEVCG_DEFAULT_ALLOW,
> + DEVCG_DEFAULT_DENY,
> + } behavior;
> };
>
> static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s)
> @@ -182,13 +185,13 @@ static struct cgroup_subsys_state *devcg
> parent_cgroup = cgroup->parent;
>
> if (parent_cgroup == NULL)
> - dev_cgroup->deny_all = false;
> + 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->deny_all = parent_dev_cgroup->deny_all;
> + dev_cgroup->behavior = parent_dev_cgroup->behavior;
> mutex_unlock(&devcgroup_mutex);
> if (ret) {
> kfree(dev_cgroup);
> @@ -260,7 +263,7 @@ static int devcgroup_seq_read(struct cgr
> * - List the exceptions in case the default policy is to deny
> * This way, the file remains as a "whitelist of devices"
> */
> - if (devcgroup->deny_all == false) {
> + if (devcgroup->behavior == DEVCG_DEFAULT_ALLOW) {
> set_access(acc, ACC_MASK);
> set_majmin(maj, ~0);
> set_majmin(min, ~0);
> @@ -314,12 +317,12 @@ if (ex->minor != ~0 && ex->minor != re
> * 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
> - * (!deny_all, !match)
> + * (behavior == DEVCG_DEFAULT_ALLOW, !match)
> * - the dev cgroup has its default policy to deny + exception list:
> * the new exception *should* match the exceptions
> - * (deny_all, match)
> + * (behavior == DEVCG_DEFAULT_DENY, match)
> */
> - if (dev_cgroup->deny_all == match)
> + if ((dev_cgroup->behavior == DEVCG_DEFAULT_DENY) == match)
> return 1;
> return 0;
> }
> @@ -375,11 +378,11 @@ memset(&ex, 0, sizeof(ex));
> if (!parent_has_perm(devcgroup, &ex))
> return -EPERM;
> dev_exception_clean(devcgroup);
> - devcgroup->deny_all = false;
> + devcgroup->behavior = DEVCG_DEFAULT_ALLOW;
> break;
> case DEVCG_DENY:
> dev_exception_clean(devcgroup);
> - devcgroup->deny_all = true;
> + devcgroup->behavior = DEVCG_DEFAULT_DENY;
> break;
> default:
> return -EINVAL;
> @@ -452,7 +455,7 @@ case '\0':
> * an matching exception instead. And be silent about it: we
> * don't want to break compatibility
> */
> - if (devcgroup->deny_all == false) {
> + if (devcgroup->behavior == DEVCG_DEFAULT_ALLOW) {
> dev_exception_rm(devcgroup, &ex);
> return 0;
> }
> @@ -463,7 +466,7 @@ return 0;
> * an matching exception instead. And be silent about it: we
> * don't want to break compatibility
> */
> - if (devcgroup->deny_all == true) {
> + if (devcgroup->behavior == DEVCG_DEFAULT_DENY) {
> dev_exception_rm(devcgroup, &ex);
> return 0;
> }
>