[apparmor] [PATCH v1.1 2/2] libapparmor: Be consistent with the type used for buffer sizes

[Tyler Hicks]

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 <tyhicks at canonical.com>
---

* 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 };
 
-- 
2.9.3