[apparmor] [PATCH v2 18/42] parser: Begin to flesh out library interface for features

Tyler Hicks tyhicks at canonical.com
Fri Mar 6 21:48:34 UTC 2015


The aa_features_new_*() functions create an aa_features object. They can
be thought of as the constructor of aa_features objects. A number of
constructors are available depending on whether the features are coming
from a file in the policy cache, a string specified on the command line,
or from apparmorfs.

The aa_features_ref() and aa_features_unref() functions are used to grab
and give up references to an aa_features. When the ref count hits zero,
all allocated memory is freed. Like with free(), aa_features_unref() can
be called with a NULL pointer for convenience.

Pre-processor macros are hidden behind functions so that they don't
become part of our ABI when we move this code into libapparmor later on.

A temporary convenience function, aa_features_get_string(), is provided
while code that uses aa_features is migrated from expecting raw features
string access to something more abstract. The function will be removed
in an upcoming patch.

Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
---
 parser/features.c     | 170 ++++++++++++++++++++++++++++++++++++++++----------
 parser/features.h     |  15 ++---
 parser/parser_main.c  |  36 ++++++-----
 parser/policy_cache.c |  23 ++++---
 parser/policy_cache.h |   4 +-
 5 files changed, 183 insertions(+), 65 deletions(-)

diff --git a/parser/features.c b/parser/features.c
index a2c25c2..ccc238d 100644
--- a/parser/features.c
+++ b/parser/features.c
@@ -33,11 +33,17 @@
 #include "lib.h"
 #include "parser.h"
 
-#define FEATURES_STRING_SIZE 8192
-char *features_string = NULL;
+#define FEATURES_FILE "/sys/kernel/security/" MODULE_NAME "/features"
+
+#define STRING_SIZE 8192
+
+struct aa_features {
+	unsigned int ref_count;
+	char string[STRING_SIZE];
+};
 
 struct features_struct {
-	char **buffer;
+	char *buffer;
 	int size;
 	char *pos;
 };
@@ -45,7 +51,7 @@ struct features_struct {
 static int features_snprintf(struct features_struct *fst, const char *fmt, ...)
 {
 	va_list args;
-	int i, remaining = fst->size - (fst->pos - *fst->buffer);
+	int i, remaining = fst->size - (fst->pos - fst->buffer);
 
 	if (remaining < 0) {
 		errno = EINVAL;
@@ -86,7 +92,7 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
 	if (S_ISREG(st->st_mode)) {
 		autoclose int file = -1;
 		int len;
-		int remaining = fst->size - (fst->pos - *fst->buffer);
+		int remaining = fst->size - (fst->pos - fst->buffer);
 
 		file = openat(dirfd(dir), name, O_RDONLY);
 		if (file == -1) {
@@ -122,7 +128,7 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
 	return 0;
 }
 
-static char *handle_features_dir(const char *filename, char **buffer, int size,
+static char *handle_features_dir(const char *filename, char *buffer, int size,
 				 char *pos)
 {
 	struct features_struct fst = { buffer, size, pos };
@@ -135,49 +141,145 @@ static char *handle_features_dir(const char *filename, char **buffer, int size,
 	return fst.pos;
 }
 
-char *load_features_file(const char *name) {
-	char *buffer;
+static int load_features_file(const char *name, char *buffer, size_t size)
+{
 	autofclose FILE *f = NULL;
-	size_t size;
+	size_t end;
 
 	f = fopen(name, "r");
 	if (!f)
-		return NULL;
-
-	buffer = (char *) malloc(FEATURES_STRING_SIZE);
-	if (!buffer)
-		goto fail;
-
-	size = fread(buffer, 1, FEATURES_STRING_SIZE - 1, f);
-	if (!size || ferror(f))
-		goto fail;
-	buffer[size] = 0;
+		return -1;
 
-	return buffer;
+	errno = 0;
+	end = fread(buffer, 1, size - 1, f);
+	if (ferror(f)) {
+		if (!errno)
+			errno = EIO;
+		return -1;
+	}
+	buffer[end] = 0;
 
-fail:
-	int save = errno;
-	free(buffer);
-	errno = save;
-	return NULL;
+	return 0;
 }
 
-int load_features(const char *name)
+/**
+ * aa_features_new - create a new features based on a path
+ * @features: will point to the address of an allocated and initialized
+ *            aa_features object upon success
+ * @path: path to a features file or directory
+ *
+ * Returns: 0 on success, -1 on error with errno set and *@features pointing to
+ *          NULL
+ */
+int aa_features_new(aa_features **features, const char *path)
 {
 	struct stat stat_file;
+	aa_features *f;
 
-	if (stat(name, &stat_file) == -1)
+	*features = NULL;
+
+	if (stat(path, &stat_file) == -1)
+		return -1;
+
+	f = (aa_features *) calloc(1, sizeof(*f));
+	if (!f) {
+		errno = ENOMEM;
 		return -1;
+	}
+	aa_features_ref(f);
 
 	if (S_ISDIR(stat_file.st_mode)) {
-		/* if we have a features directory default to */
-		features_string = (char *) malloc(FEATURES_STRING_SIZE);
-		handle_features_dir(name, &features_string, FEATURES_STRING_SIZE, features_string);
-	} else {
-		features_string = load_features_file(name);
-		if (!features_string)
-			return -1;
+		handle_features_dir(path, f->string, STRING_SIZE, f->string);
+	} else if (load_features_file(path, f->string, STRING_SIZE)) {
+		int save = errno;
+
+		aa_features_unref(f);
+		errno = save;
+		return -1;
+	}
+
+	*features = f;
+
+	return 0;
+}
+
+/**
+ * aa_features_new_from_string - create a new features based on a string
+ * @features: will point to the address of an allocated and initialized
+ *            aa_features object upon success
+ * @string: a NUL-terminated string representation of features
+ * @size: the size of @string, not counting the NUL-terminator
+ *
+ * Returns: 0 on success, -1 on error with errno set and *@features pointing to
+ *          NULL
+ */
+int aa_features_new_from_string(aa_features **features,
+				const char *string, size_t size)
+{
+	aa_features *f;
+
+	*features = NULL;
+
+	/* Require size to be less than STRING_SIZE so there's room for a NUL */
+	if (size >= STRING_SIZE)
+		return ENOBUFS;
+
+	f = (aa_features *) calloc(1, sizeof(*f));
+	if (!f) {
+		errno = ENOMEM;
+		return -1;
 	}
+	aa_features_ref(f);
+
+	memcpy(f->string, string, size);
+	f->string[size] = '\0';
+	*features = f;
 
 	return 0;
 }
+
+/**
+ * aa_features_new_from_kernel - create a new features based on the current kernel
+ * @features: will point to the address of an allocated and initialized
+ *            aa_features object upon success
+ *
+ * Returns: 0 on success, -1 on error with errno set and *@features pointing to
+ *          NULL
+ */
+int aa_features_new_from_kernel(aa_features **features)
+{
+	return aa_features_new(features, FEATURES_FILE);
+}
+
+/**
+ * aa_features_ref - increments the ref count of a features
+ * @features: the features
+ *
+ * Returns: the features
+ */
+aa_features *aa_features_ref(aa_features *features)
+{
+	atomic_inc(&features->ref_count);
+	return features;
+}
+
+/**
+ * aa_features_unref - decrements the ref count and frees the features when 0
+ * @features: the features (can be NULL)
+ */
+void aa_features_unref(aa_features *features)
+{
+	if (features && atomic_dec_and_test(&features->ref_count))
+		free(features);
+}
+
+/**
+ * aa_features_get_string - provides immutable string representation of features
+ * @features: the features
+ *
+ * Returns: an immutable string representation of features
+ */
+const char *aa_features_get_string(aa_features *features)
+{
+	return features->string;
+}
diff --git a/parser/features.h b/parser/features.h
index 201d8fc..100f460 100644
--- a/parser/features.h
+++ b/parser/features.h
@@ -19,13 +19,14 @@
 #ifndef __AA_FEATURES_H
 #define __AA_FEATURES_H
 
-#include "parser.h"
+typedef struct aa_features aa_features;
 
-#define FEATURES_FILE "/sys/kernel/security/" MODULE_NAME "/features"
-
-extern char *features_string;
-
-char *load_features_file(const char *name);
-int load_features(const char *name);
+int aa_features_new(aa_features **features, const char *path);
+int aa_features_new_from_string(aa_features **features,
+				const char *string, size_t size);
+int aa_features_new_from_kernel(aa_features **features);
+aa_features *aa_features_ref(aa_features *features);
+void aa_features_unref(aa_features *features);
+const char *aa_features_get_string(aa_features *features);
 
 #endif /* __AA_FEATURES_H */
diff --git a/parser/parser_main.c b/parser/parser_main.c
index 2455103..3f8ca6a 100644
--- a/parser/parser_main.c
+++ b/parser/parser_main.c
@@ -82,6 +82,8 @@ struct timespec mru_tstamp;
 
 static char *cacheloc = NULL;
 
+static aa_features *features = NULL;
+
 /* Make sure to update BOTH the short and long_options */
 static const char *short_options = "adf:h::rRVvI:b:BCD:NSm:M:qQn:XKTWkL:O:po:";
 struct option long_options[] = {
@@ -389,11 +391,17 @@ static int process_arg(int c, char *optarg)
 		}
 		break;
 	case 'm':
-		features_string = strdup(optarg);
+		if (aa_features_new_from_string(&features,
+						optarg, strlen(optarg))) {
+			fprintf(stderr,
+				"Failed to parse features string: %m\n");
+			exit(1);
+		}
 		break;
 	case 'M':
-		if (load_features(optarg) == -1) {
-			fprintf(stderr, "Failed to load features from '%s'\n",
+		if (aa_features_new(&features, optarg)) {
+			fprintf(stderr,
+				"Failed to load features from '%s': %m\n",
 				optarg);
 			exit(1);
 		}
@@ -564,16 +572,17 @@ no_match:
 	perms_create = 1;
 }
 
-static void set_supported_features(void) {
+static void set_supported_features(void)
+{
+	const char *features_string;
 
 	/* has process_args() already assigned a match string? */
-	if (!features_string) {
-		if (load_features(FEATURES_FILE) == -1) {
-			set_features_by_match_file();
-			return;
-		}
+	if (!features && aa_features_new_from_kernel(&features) == -1) {
+		set_features_by_match_file();
+		return;
 	}
 
+	features_string = aa_features_get_string(features);
 	perms_create = 1;
 
 	/* TODO: make this real parsing and config setting */
@@ -865,10 +874,9 @@ static void setup_flags(void)
 	set_supported_features();
 
 	/* Gracefully handle AppArmor kernel without compatibility patch */
-	if (!features_string) {
-		PERROR("Cache read/write disabled: %s interface file missing. "
-			"(Kernel needs AppArmor 2.4 compatibility patch.)\n",
-			FEATURES_FILE);
+	if (!features) {
+		PERROR("Cache read/write disabled: interface file missing. "
+			"(Kernel needs AppArmor 2.4 compatibility patch.)\n");
 		write_cache = 0;
 		skip_read_cache = 1;
 		return;
@@ -924,7 +932,7 @@ int main(int argc, char *argv[])
 		return 0;
 	}
 
-	retval = setup_cache(cacheloc);
+	retval = setup_cache(features, cacheloc);
 	if (retval) {
 		PERROR(_("Failed setting up policy cache (%s): %s\n"),
 		       cacheloc, strerror(errno));
diff --git a/parser/policy_cache.c b/parser/policy_cache.c
index 469d884..8d34f34 100644
--- a/parser/policy_cache.c
+++ b/parser/policy_cache.c
@@ -30,7 +30,6 @@
 #define _(s) gettext(s)
 
 #include "lib.h"
-#include "features.h"
 #include "parser.h"
 #include "policy_cache.h"
 
@@ -228,10 +227,11 @@ void install_cache(const char *cachetmpname, const char *cachename)
 	}
 }
 
-int setup_cache(const char *cacheloc)
+int setup_cache(aa_features *kernel_features, const char *cacheloc)
 {
 	autofree char *cache_features_path = NULL;
-	autofree char *cache_flags = NULL;
+	aa_features *cache_features;
+	const char *kernel_features_string;
 
 	if (!cacheloc) {
 		errno = EINVAL;
@@ -250,22 +250,27 @@ int setup_cache(const char *cacheloc)
 		return -1;
 	}
 
-	cache_flags = load_features_file(cache_features_path);
-	if (cache_flags) {
-		if (strcmp(features_string, cache_flags) != 0) {
+	kernel_features_string = aa_features_get_string(kernel_features);
+	if (!aa_features_new(&cache_features, cache_features_path)) {
+		const char *cache_features_string;
+
+		cache_features_string = aa_features_get_string(cache_features);
+		if (strcmp(kernel_features_string, cache_features_string) != 0) {
 			if (write_cache && cond_clear_cache) {
 				if (create_cache(cacheloc, cache_features_path,
-						 features_string))
+						 kernel_features_string))
 					skip_read_cache = 1;
 			} else {
 				if (show_cache)
-					PERROR("Cache read/write disabled: %s does not match %s\n", FEATURES_FILE, cache_features_path);
+					PERROR("Cache read/write disabled: Police cache is invalid\n");
 				write_cache = 0;
 				skip_read_cache = 1;
 			}
 		}
+		aa_features_unref(cache_features);
 	} else if (write_cache) {
-		create_cache(cacheloc, cache_features_path, features_string);
+		create_cache(cacheloc, cache_features_path,
+			     kernel_features_string);
 	}
 
 	return 0;
diff --git a/parser/policy_cache.h b/parser/policy_cache.h
index 68d45a4..728de57 100644
--- a/parser/policy_cache.h
+++ b/parser/policy_cache.h
@@ -19,6 +19,8 @@
 #ifndef __AA_POLICY_CACHE_H
 #define __AA_POLICY_CACHE_H
 
+#include "features.h"
+
 extern struct timespec mru_tstamp;
 
 /* returns true if time is more recent than mru_tstamp */
@@ -46,6 +48,6 @@ void valid_read_cache(const char *cachename);
 int cache_hit(const char *cachename);
 int setup_cache_tmp(const char **cachetmpname, const char *cachename);
 void install_cache(const char *cachetmpname, const char *cachename);
-int setup_cache(const char *cacheloc);
+int setup_cache(aa_features *kernel_features, const char *cacheloc);
 
 #endif /* __AA_POLICY_CACHE_H */
-- 
2.1.4




More information about the AppArmor mailing list