Re: [PATCH v2 3/4] device_cgroup: make may_access() stronger

From: Tejun Heo
Date: Thu Jan 24 2013 - 17:13:24 EST


Hello,

On Thu, Jan 24, 2013 at 02:50:00PM -0500, aris@xxxxxxxxxx wrote:
> In order to revalidate local exceptions for the hierarchy change propagation,
> make may_access() stronger.

It would be nice to explain what "stronger" actually means.

> --- github.orig/security/device_cgroup.c 2013-01-24 10:40:46.384253615 -0500
> +++ github/security/device_cgroup.c 2013-01-24 10:41:07.513567697 -0500
> @@ -353,13 +353,15 @@ return 0;
> * 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
> + * @behavior: behavior of the exception

Should come after @refex?

> * @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,
> + enum devcg_behavior behavior)
> {
> struct dev_exception_item *ex;
> - bool match = false;
> + int match = false;
>
> rcu_lockdep_assert(rcu_read_lock_held() ||
> lockdep_is_held(&devcgroup_mutex),
> @@ -380,18 +382,28 @@ if (ex->minor != ~0 && ex->minor != re
> break;
> }
>
> - /*
> - * 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)
> - */
> - if ((dev_cgroup->behavior == DEVCG_DEFAULT_DENY) == match)
> - return 1;
> - return 0;
> + 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 {
> + /* only behavior == DEVCG_DEFAULT_DENY allowed here */
> + if (match)
> + /* parent has an exception that matches the proposed */
> + return true;
> + else
> + return false;

It would be nice if there were a separate patch to decompress the
logic separate from adding new logic.

Thanks.

--
tejun
--
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/