[apparmor] [PATCH 2/6] libapparmor: Don't count NUL terminator byte

[Tyler Hicks]

On 2015-05-12 18:11:05, Seth Arnold wrote:
> On Mon, Apr 13, 2015 at 04:56:28PM -0500, Tyler Hicks wrote:
> > When passing the size of the confinement context to
> > parse_confinement_mode(), don't include the NUL terminator byte in the
> > size.
> > 
> > It is confusing to count the NUL terminator as part of the string's
> > length. This change makes it so that, after a few additional changes,
> > parse_confinement_mode() can be exposed as part of libapparmor's public
> > API.
> 
> I think one of the cases may include the nul in the size...
> 
> > 
> > Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> > ---
> >  libraries/libapparmor/src/kernel.c | 19 +++++++++++--------
> >  1 file changed, 11 insertions(+), 8 deletions(-)
> > 
> > diff --git a/libraries/libapparmor/src/kernel.c b/libraries/libapparmor/src/kernel.c
> > index 9d5f45d..14593b7 100644
> > --- a/libraries/libapparmor/src/kernel.c
> > +++ b/libraries/libapparmor/src/kernel.c
> > @@ -154,7 +154,7 @@ static char *procattr_path(pid_t pid, const char *attr)
> >  /**
> >   * parse_confinement_mode - get the mode from the confinement context
> >   * @con: the confinement context
> > - * @size: size of the confinement context
> > + * @size: size of the confinement context (not including the NUL terminator)
> >   *
> >   * Modifies con to NUL-terminate the label string and the mode string.
> >   *
> > @@ -164,14 +164,14 @@ static char *procattr_path(pid_t pid, const char *attr)
> >  static char *parse_confinement_mode(char *con, int size)
> >  {
> >  	if (strcmp(con, "unconfined") != 0 &&
> > -	    size > 4 && con[size - 2] == ')') {
> > -		int pos = size - 3;
> > +	    size > 3 && con[size - 1] == ')') {
> > +		int pos = size - 2;
> >  
> >  		while (pos > 0 && !(con[pos] == ' ' && con[pos + 1] == '('))
> >  			pos--;
> >  		if (pos > 0) {
> >  			con[pos] = 0; /* overwrite ' ' */
> > -			con[size - 2] = 0; /* overwrite trailing ) */
> > +			con[size - 1] = 0; /* overwrite trailing ) */
> >  			return &con[pos + 2]; /* skip '(' */
> >  		}
> >  	}
> > @@ -236,18 +236,21 @@ int aa_getprocattr_raw(pid_t tid, const char *attr, char *buf, int len,
> >  		errno = saved;
> >  		goto out;
> >  	} else if (size > 0 && buf[size - 1] != 0) {
> > +		char *nul;
> > +
> >  		/* check for null termination */
> >  		if (buf[size - 1] == '\n') {
> > -			buf[size - 1] = 0;
> > +			nul = &buf[size - 1];
> >  		} else if (len == 0) {
> >  			errno = ERANGE;
> >  			goto out2;
> >  		} else {
> > -			buf[size] = 0;
> > +			nul = &buf[size];
> >  			size++;
> 
> Is this size++ now incorrect? (I was going to suggest it be deleted
> because it isn't used further in this patch, but size is used as the
> return value for the function. Perhaps the return value should also be
> amended to (nul - buf)?)

The size++ is still correct. The aa_getcon(2) family of functions
includes any NUL bytes in their return values:

  RETURN VALUE
       On success size of data placed in the buffer is returned, this includes
       the mode if present and any terminating characters. On error, -1 is
       returned, and errno(3) is set appropriately.

This patch only changes the size passed into parse_confinement_mode(). The
return value of aa_getprocattr_raw() intentionally still counts the NUL bytes
read into buf.

Tyler

> 
> >  		}
> >  
> > -		mode_str = parse_confinement_mode(buf, size);
> > +		*nul = 0;
> > +		mode_str = parse_confinement_mode(buf, nul - buf);
> >  		if (mode)
> >  			*mode = mode_str;
> >  	}
> > @@ -614,7 +617,7 @@ int aa_getpeercon_raw(int fd, char *buf, int *len, char **mode)
> >  		}
> >  	}
> >  
> > -	mode_str = parse_confinement_mode(buf, optlen);
> > +	mode_str = parse_confinement_mode(buf, optlen - 1);
> >  	if (mode)
> >  		*mode = mode_str;
> >  
> 
> Thanks



> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150514/3225a54c/attachment.pgp>