[apparmor] [PATCH 4/5] Add the ability to read proc attr interfaces

[Steve Beattie]

On Tue, Jul 12, 2011 at 01:50:22PM -0700, John Johansen wrote:
> On 07/12/2011 12:01 PM, Seth Arnold wrote:
> > The ret=1 just before the for loop isn't needed (except to quiet warnings?) 
> > I think the entire for loop would be easier read as a while loop.
> > 
> heh, sorry you were right, I missed that I was double assigning ret.
> 
> revised patch using while and fixing a couple of bugs below
> 
> ---
> 
> Add the ability to read proc attr interfaces
> 
> Signed-off-by: John Johansen <john.johansen at canonical.com>
> 
> diff --git a/libraries/libapparmor/src/kernel_interface.c b/libraries/libapparmor/src/kernel_interface.c
> index 0e61065..9f16bd0 100644
> --- a/libraries/libapparmor/src/kernel_interface.c
> +++ b/libraries/libapparmor/src/kernel_interface.c
> @@ -54,6 +54,68 @@ static char *procattr_path(pid_t pid, const char *attr)
>  	return NULL;
>  }
>  
> +/**
> + * getprocattr - get the contents of @attr for @tid into @buf
> + * @tid: tid of task to query
> + * @attr: which /proc/<tid>/current/attr to query

Nitpick: "which /proc/<tid>/attr/<attr> to query"

> + * @buf: buffer to store the result in
> + * @len: size of the buffer
> + *
> + * Returns: size of data read or -1 on error, and sets errno
> + */
> +static int getprocattr(pid_t tid, const char *attr, char *buf, int len)
> +{
> +	int rc = -1;
> +	int fd, ret;
> +	char *ctl = NULL;
> +	int size = 0;
> +
> +	if (!buf || len <= 0) {
> +		errno = EINVAL;
> +		goto out;
> +	}
> +
> +	ctl = procattr_path(tid, attr);
> +	if (!ctl)
> +		goto out;
> +
> +	fd = open(ctl, O_RDONLY);
> +	if (fd == -1) {
> +		goto out;
> +	}
> +
> +	do {
> +		ret = read(fd, buf, len);
> +		if (ret <= 0)
> +			break;
> +		buf += ret;
> +		size += ret;
> +		len -= ret;
> +		if (len < 0) {
> +			errno = ENAMETOOLONG;
> +			break;

Is the intent to require the caller to check both errno and the return
value rc? Because with the current code, if the buffer is too small, rc
is still set to the amount read. You can see this with the attached
patch to the aa_logmisc.c unit tests (I had to make getprocattr not
static for it to work).

> +		}
> +	} while (ret > 0);
> +
> +	if (ret < 0) {
> +		int saved;
> +		if (ret != -1) {
> +			errno = EPROTO;
> +		}
> +		saved = errno;
> +		(void)close(fd);
> +		errno = saved;
> +		goto out;
> +	}
> +
> +	(void)close(fd);
> +
> +	rc = size;
> +out:
> +	free(ctl);
> +	return rc;
> +}
> +
>  static int setprocattr(pid_t tid, const char *attr, const char *buf, int len)
>  {
>  	int rc = -1;
> 

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unit_test.patch
Type: text/x-diff
Size: 1189 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20110714/d834943d/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20110714/d834943d/attachment.pgp>