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 <[email protected]> > > --- > > 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
> [email protected]
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
