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