linux-kernel - Re: [PATCH v2 3/4] device_cgroup: make may

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130124221327.GX2373@mtj.dyndns.org>
Date:	Thu, 24 Jan 2013 14:13:27 -0800
From:	Tejun Heo <tj@...nel.org>
To:	aris@...hat.com
Cc:	linux-kernel@...r.kernel.org, cgroups@...r.kernel.org,
	Serge Hallyn <serge.hallyn@...onical.com>
Subject: Re: [PATCH v2 3/4] device_cgroup: make may_access() stronger

Hello,

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