On Fri, Sep 30, 2016 at 02:07:28PM -0500, Tyler Hicks wrote: > The features_struct.size variable is used to hold a buffer size and it > is also passed in as the size parameter to read(). It should be a size_t > instead of an int. > > A new helper function, features_buffer_remaining(), is created to handle > the two places where the remaining bytes in the features buffer are > calculated. > > This patch also changes the size parameter of load_features_dir() to a > size_t to match the same parameter of load_features_file() as well as > the features_struct.size change described above. > > Two casts were needed when comparing signed types to unsigned types. > These casts are safe because the signed value is checked for "< 0" > immediately before the casts. > > Signed-off-by: Tyler Hicks <[email protected]>
I'm not a big fan of the casts but I'm not sure C leaves us much choice. The pre-cast checks are nice. Acked-by: Seth Arnold <[email protected]> Thanks > --- > > * Changes since v1: > - Subtract fst->buffer from fst->pos and ensure the result is not greater > than remaining before subtracting > - Move the remaining buffer calculation into a helper function > - Use the helper function in features_dir_cb(), which didn't have a sanity > check on the value of remaining > > libraries/libapparmor/src/features.c | 35 ++++++++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/libraries/libapparmor/src/features.c > b/libraries/libapparmor/src/features.c > index 088c4ea..c656c37 100644 > --- a/libraries/libapparmor/src/features.c > +++ b/libraries/libapparmor/src/features.c > @@ -23,6 +23,7 @@ > #include <stdio.h> > #include <string.h> > #include <stdarg.h> > +#include <stddef.h> > #include <stdlib.h> > #include <sys/types.h> > #include <sys/stat.h> > @@ -42,7 +43,7 @@ struct aa_features { > > struct features_struct { > char *buffer; > - int size; > + size_t size; > char *pos; > }; > > @@ -51,17 +52,30 @@ struct component { > size_t len; > }; > > -static int features_snprintf(struct features_struct *fst, const char *fmt, > ...) > +static int features_buffer_remaining(struct features_struct *fst, > + size_t *remaining) > { > - va_list args; > - int i, remaining = fst->size - (fst->pos - fst->buffer); > + ptrdiff_t offset = fst->pos - fst->buffer; > > - if (remaining < 0) { > + if (offset < 0 || fst->size < (size_t)offset) { > errno = EINVAL; > - PERROR("Invalid features buffer offset\n"); > + PERROR("Invalid features buffer offset (%td)\n", offset); > return -1; > } > > + *remaining = fst->size - offset; > + return 0; > +} > + > +static int features_snprintf(struct features_struct *fst, const char *fmt, > ...) > +{ > + va_list args; > + int i; > + size_t remaining; > + > + if (features_buffer_remaining(fst, &remaining) == -1) > + return -1; > + > va_start(args, fmt); > i = vsnprintf(fst->pos, remaining, fmt, args); > va_end(args); > @@ -70,7 +84,7 @@ static int features_snprintf(struct features_struct *fst, > const char *fmt, ...) > errno = EIO; > PERROR("Failed to write to features buffer\n"); > return -1; > - } else if (i >= remaining) { > + } else if ((size_t)i >= remaining) { > errno = ENOBUFS; > PERROR("Feature buffer full."); > return -1; > @@ -157,7 +171,10 @@ static int features_dir_cb(int dirfd, const char *name, > struct stat *st, > > if (S_ISREG(st->st_mode)) { > ssize_t len; > - int remaining = fst->size - (fst->pos - fst->buffer); > + size_t remaining; > + > + if (features_buffer_remaining(fst, &remaining) == -1) > + return -1; > > len = load_features_file(dirfd, name, fst->pos, remaining); > if (len < 0) > @@ -176,7 +193,7 @@ static int features_dir_cb(int dirfd, const char *name, > struct stat *st, > } > > static ssize_t load_features_dir(int dirfd, const char *path, > - char *buffer, int size) > + char *buffer, size_t size) > { > struct features_struct fst = { buffer, size, buffer }; >
signature.asc
Description: PGP signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
