Re: [PATCH v9 1/2] LSM: Multiple concurrent LSMs

[Casey Schaufler <casey@schaufler-ca.com>]

On 12/4/2012 5:43 AM, Tetsuo Handa wrote:
> Below is an approach for allowing the callers to determine how many of
> /name=value/ pairs passed to setprocattr() have succeeded.
> Returns -ve if none processed and returns offset if at least one processed.
> (Well, should we use '\0' as separater rather than '/' ?)

I don't want to use '\0' because then you can't use:

	echo "/smack=foo/apparmor=/usr/bin/cupsd (enforce)/" > /proc/self/attr/current

I have however identified where my scheme is flawed. If you did the above when
AppArmor is not configured you get a smack label of "foo/apparmor=/usr/bin/cupsd".
So it has to change. What do y'all think of:

	<smack>foo</smack><apparmor>/usr/bin/cupsd (enforce)</apparmor>

Yeah, it's verbose but it wouldn't be any worse to parse than what I have now
and it addresses the problem.

Or, offer a solution you like better, but it needs to be plain text, no embedded
null characters.


>
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2061,66 +2061,51 @@ int security_setprocattr(struct task_struct *p, char *name, void *value,
>  				size_t size)
>  {
>  	struct security_operations *sop;
> -	char searches[COMPOSER_MAX][SECURITY_NAME_MAX + 4];
>  	char *data = value;
> -	char *values[COMPOSER_MAX];
> -	int eos = 0;
> -	int formatted = 0;
> -	int only = 0;
> -	int thisrc;
> -	int rc = size;
> -
> -	if (lsm_present)
> -		return present_setprocattr(p, name, value, size);
> +	char * const start = data;
> +	int processed = 0;
> +	int rc = -EINVAL;
>  
> -	if (size > PAGE_SIZE - 1)
> +	if (size && data[0] != '/') {
> +		if (lsm_present)
> +			return present_setprocattr(p, name, value, size);
>  		return -EINVAL;
> -	if (size > 0)
> -		eos = size - 1;
> -	while (eos > 0 && (data[eos] == '\0' || data[eos] == '\n'))
> -		eos--;
> -
> -	if (data[0] == '/' && data[eos] == '/') {
> -		/*
> -		 * "/smack=Howdy/apparmor=confined/bandl=12/1,2,6/"
> -		 */
> -		formatted = 1;
> -		data[eos] = '\0';
> -		for_each_hook(sop, setprocattr) {
> -			sprintf(searches[sop->order], "/%s=", sop->name);
> -			values[sop->order] = strnstr(data,
> -						searches[sop->order], size);
> -		}
> -		for_each_hook(sop, setprocattr) {
> -			if (values[sop->order] != NULL) {
> -				*values[sop->order] = '\0';
> -				values[sop->order] = values[sop->order] +
> -					strlen(searches[sop->order]);
> -			}
> -		}
>  	}
> +	/*
> +	 * "/smack=Howdy/apparmor=confined/bandl=12/1,2,6/"
> +	 *
> +	 * Parse and dispatch sequentially, and use fail and bail so that
> +	 * the caller can determine that setprocattr() has partially
> +	 * succeeded via "return value" less than "size" argument.
> +	 */
> +next:
>  	for_each_hook(sop, setprocattr) {
> -		if (formatted && values[sop->order] == NULL)
> +		const char *cp = sop->name;
> +		size_t len = strlen(cp);
> +		if (size < len + 2 || memcmp(data + 1, cp, len) ||
> +		    data[len + 1] != '=')
>  			continue;
> -		if (only == 0)
> -			only = sop->order;
> -		else
> -			only = -1;
> -
> -		if (formatted)
> -			thisrc = sop->setprocattr(p, name, values[sop->order],
> -					strlen(values[sop->order]) + 1);
> -		else
> -			thisrc = sop->setprocattr(p, name, value, size);
> -
> -		if (thisrc < 0)
> -			rc = thisrc;
> +		len += 2;
> +		data += len;
> +		size -= len;
> +		cp = memchr(data, '/', size);
> +		if (!cp || cp == data)
> +			break;
> +		len = cp - data;
> +		rc = sop->setprocattr(p, name, data, len);
> +		if (rc < 0)
> +			break;
> +		rc = -EINVAL;
> +		data = (char *) cp;
> +		size -= len;
> +		processed = data - start + 1;
> +		if (size > 1 && data[0] == '/')
> +			goto next;
> +		break;
>  	}
> -
> -	if (only == 0)
> -		return -EINVAL;
> +	if (processed)
> +		return processed;
>  	return rc;
> -
>  }
>  
>  int security_netlink_send(struct sock *sk, struct sk_buff *skb)
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.