Hello,

On Thu, Jan 24, 2013 at 02:50:00PM -0500, [email protected] 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to