snprintf_buffer() needed to be modified in order to properly return error
conditions up the stack, instead of exiting, but there were some other
cleanups that it could use.

It was obviously implemented with the features_struct in mind so this
patch simplifies the input parameters by directly accepting a
features_struct pointer. Also, the name is changed to reflect that it is
intended to work on a features_struct instead of an arbritrary buffer.

A quick sanity check is added to make sure that the features_struct.pos
value isn't pointing past the end of the buffer.

The printf(3) family of functions can return a negative value upon error
so a check of the return value of vsnprintf(3) is added.

Finally, the return values of the function are simplified to 0 on
success or -1, with errno set, on error. This is possible since
features_struct.pos can be internally updated after a successful
vsnprintf(3).

Signed-off-by: Tyler Hicks <tyhi...@canonical.com>
---
 parser/features.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/parser/features.c b/parser/features.c
index 673fe4f..a2c25c2 100644
--- a/parser/features.c
+++ b/parser/features.c
@@ -36,30 +36,41 @@
 #define FEATURES_STRING_SIZE 8192
 char *features_string = NULL;
 
-static char *snprintf_buffer(char *buf, char *pos, ssize_t size,
-                            const char *fmt, ...)
+struct features_struct {
+       char **buffer;
+       int size;
+       char *pos;
+};
+
+static int features_snprintf(struct features_struct *fst, const char *fmt, ...)
 {
        va_list args;
-       int i, remaining = size - (pos - buf);
+       int i, remaining = fst->size - (fst->pos - *fst->buffer);
+
+       if (remaining < 0) {
+               errno = EINVAL;
+               PERROR(_("Invalid features buffer offset\n"));
+               return -1;
+       }
 
        va_start(args, fmt);
-       i = vsnprintf(pos, remaining, fmt, args);
+       i = vsnprintf(fst->pos, remaining, fmt, args);
        va_end(args);
 
-       if (i >= size) {
+       if (i < 0) {
+               errno = EIO;
+               PERROR(_("Failed to write to features buffer\n"));
+               return -1;
+       } else if (i >= remaining) {
+               errno = ENOBUFS;
                PERROR(_("Feature buffer full."));
-               exit(1);
+               return -1;
        }
 
-       return pos + i;
+       fst->pos += i;
+       return 0;
 }
 
-struct features_struct {
-       char **buffer;
-       int size;
-       char *pos;
-};
-
 static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
                           void *data)
 {
@@ -69,7 +80,8 @@ static int features_dir_cb(DIR *dir, const char *name, struct 
stat *st,
        if (*name == '.' || !strlen(name))
                return 0;
 
-       fst->pos = snprintf_buffer(*fst->buffer, fst->pos, fst->size, "%s {", 
name);
+       if (features_snprintf(fst, "%s {", name) == -1)
+               return -1;
 
        if (S_ISREG(st->st_mode)) {
                autoclose int file = -1;
@@ -104,7 +116,8 @@ static int features_dir_cb(DIR *dir, const char *name, 
struct stat *st,
                        return -1;
        }
 
-       fst->pos = snprintf_buffer(*fst->buffer, fst->pos, fst->size, "}\n");
+       if (features_snprintf(fst, "}\n") == -1)
+               return -1;
 
        return 0;
 }
-- 
2.1.4


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

Reply via email to