On 04/02/2015 08:17 AM, Tyler Hicks wrote: > The aa_policy_cache_new() and aa_policy_cache_remove() functions are > changed to accept a dirfd parameter. > > The cache dirfd (by default, /etc/apparmor.d/cache) is opened earlier in > aa_policy_cache_new(). Previously, the directory wasn't accessed until > later in the following call chain: > > aa_policy_cache_new() -> init_cache_features() -> create_cache() > > Because of this change, the logic to create the cache dir must be moved > from create_cache() to aa_policy_cache_new(). > > Signed-off-by: Tyler Hicks <[email protected]>
Acked-by: John Johansen <[email protected]> > --- > libraries/libapparmor/include/sys/apparmor.h | 6 +- > libraries/libapparmor/src/policy_cache.c | 91 > +++++++++++----------------- > parser/parser_main.c | 6 +- > tests/regression/apparmor/aa_policy_cache.c | 8 ++- > 4 files changed, 48 insertions(+), 63 deletions(-) > > diff --git a/libraries/libapparmor/include/sys/apparmor.h > b/libraries/libapparmor/include/sys/apparmor.h > index 8fc3bda..ca75e5a 100644 > --- a/libraries/libapparmor/include/sys/apparmor.h > +++ b/libraries/libapparmor/include/sys/apparmor.h > @@ -142,12 +142,12 @@ int aa_kernel_interface_write_policy(int fd, const char > *buffer, size_t size); > > typedef struct aa_policy_cache aa_policy_cache; > int aa_policy_cache_new(aa_policy_cache **policy_cache, > - aa_features *kernel_features, const char *path, > - uint16_t max_caches); > + aa_features *kernel_features, > + int dirfd, const char *path, uint16_t max_caches); > aa_policy_cache *aa_policy_cache_ref(aa_policy_cache *policy_cache); > void aa_policy_cache_unref(aa_policy_cache *policy_cache); > > -int aa_policy_cache_remove(const char *path); > +int aa_policy_cache_remove(int dirfd, const char *path); > int aa_policy_cache_replace_all(aa_policy_cache *policy_cache, > aa_kernel_interface *kernel_interface); > > diff --git a/libraries/libapparmor/src/policy_cache.c > b/libraries/libapparmor/src/policy_cache.c > index f59198a..04086e7 100644 > --- a/libraries/libapparmor/src/policy_cache.c > +++ b/libraries/libapparmor/src/policy_cache.c > @@ -28,12 +28,13 @@ > > #include "private.h" > > +#define CACHE_FEATURES_FILE ".features" > + > struct aa_policy_cache { > unsigned int ref_count; > aa_features *features; > aa_features *kernel_features; > - char *path; > - char *features_path; > + int dirfd; > }; > > static int clear_cache_cb(int dirfd, const char *path, struct stat *st, > @@ -49,37 +50,16 @@ static int clear_cache_cb(int dirfd, const char *path, > struct stat *st, > > static int create_cache(aa_policy_cache *policy_cache, aa_features *features) > { > - struct stat stat_file; > - autofclose FILE * f = NULL; > - > - if (aa_policy_cache_remove(policy_cache->path)) > - goto error; > + if (aa_policy_cache_remove(policy_cache->dirfd, ".")) > + return -1; > > -create_file: > - if (aa_features_write_to_file(features, -1, > - policy_cache->features_path) == -1) > - goto error; > + if (aa_features_write_to_file(features, policy_cache->dirfd, > + CACHE_FEATURES_FILE) == -1) > + return -1; > > aa_features_unref(policy_cache->features); > policy_cache->features = aa_features_ref(features); > return 0; > - > -error: > - /* does the dir exist? */ > - if (stat(policy_cache->path, &stat_file) == -1) { > - if (mkdir(policy_cache->path, 0700) == 0) > - goto create_file; > - PERROR("Can't create cache directory: %s\n", > - policy_cache->path); > - } else if (!S_ISDIR(stat_file.st_mode)) { > - PERROR("File in cache directory location: %s\n", > - policy_cache->path); > - } else { > - PERROR("Can't update cache directory: %s\n", > - policy_cache->path); > - } > - > - return -1; > } > > static int init_cache_features(aa_policy_cache *policy_cache, > @@ -87,8 +67,8 @@ static int init_cache_features(aa_policy_cache > *policy_cache, > { > bool call_create_cache = false; > > - if (aa_features_new(&policy_cache->features, -1, > - policy_cache->features_path)) { > + if (aa_features_new(&policy_cache->features, policy_cache->dirfd, > + CACHE_FEATURES_FILE)) { > policy_cache->features = NULL; > if (!create || errno != ENOENT) > return -1; > @@ -122,17 +102,11 @@ static int replace_all_cb(int dirfd unused, const char > *name, struct stat *st, > > if (!S_ISDIR(st->st_mode) && !_aa_is_blacklisted(name, NULL)) { > struct replace_all_cb_data *data; > - autofree char *path = NULL; > > data = (struct replace_all_cb_data *) cb_data; > - if (asprintf(&path, "%s/%s", > - data->policy_cache->path, name) < 0) { > - path = NULL; > - errno = ENOMEM; > - return -1; > - } > retval = > aa_kernel_interface_replace_policy_from_file(data->kernel_interface, > - -1, path); > + > data->policy_cache->dirfd, > + name); > } > > return retval; > @@ -144,6 +118,7 @@ static int replace_all_cb(int dirfd unused, const char > *name, struct stat *st, > * aa_policy_cache_new object upon success > * @kernel_features: features representing a kernel (may be NULL if you want > to > * use the features of the currently running kernel) > + * @dirfd: directory file descriptor or AT_FDCWD (see openat(2)) > * @path: path to the policy cache > * @max_caches: The maximum number of policy caches, one for each unique set > of > * kernel features, before older caches are auto-reaped. 0 means > @@ -155,8 +130,8 @@ static int replace_all_cb(int dirfd unused, const char > *name, struct stat *st, > * pointing to NULL > */ > int aa_policy_cache_new(aa_policy_cache **policy_cache, > - aa_features *kernel_features, const char *path, > - uint16_t max_caches) > + aa_features *kernel_features, > + int dirfd, const char *path, uint16_t max_caches) > { > aa_policy_cache *pc; > bool create = max_caches > 0; > @@ -178,19 +153,26 @@ int aa_policy_cache_new(aa_policy_cache **policy_cache, > errno = ENOMEM; > return -1; > } > + pc->dirfd = -1; > aa_policy_cache_ref(pc); > > - pc->path = strdup(path); > - if (!pc->path) { > - aa_policy_cache_unref(pc); > - errno = ENOMEM; > - return -1; > - } > +open: > + pc->dirfd = openat(dirfd, path, O_RDONLY | O_CLOEXEC | O_DIRECTORY); > + if (pc->dirfd < 0) { > + int save; > + > + /* does the dir exist? */ > + if (create && errno == ENOENT) { > + if (mkdirat(dirfd, path, 0700) == 0) > + goto open; > + PERROR("Can't create cache directory '%s': %m\n", path); > + } else { > + PERROR("Can't update cache directory '%s': %m\n", path); > + } > > - if (asprintf(&pc->features_path, "%s/.features", pc->path) == -1) { > - pc->features_path = NULL; > + save = errno; > aa_policy_cache_unref(pc); > - errno = ENOMEM; > + errno = save; > return -1; > } > > @@ -239,21 +221,22 @@ void aa_policy_cache_unref(aa_policy_cache > *policy_cache) > if (policy_cache && atomic_dec_and_test(&policy_cache->ref_count)) { > aa_features_unref(policy_cache->features); > aa_features_unref(policy_cache->kernel_features); > - free(policy_cache->features_path); > - free(policy_cache->path); > + if (policy_cache->dirfd != -1) > + close(policy_cache->dirfd); > free(policy_cache); > } > } > > /** > * aa_policy_cache_remove - removes all policy cache files under a path > + * @dirfd: directory file descriptor or AT_FDCWD (see openat(2)) > * @path: the path to a policy cache directory > * > * Returns: 0 on success, -1 on error with errno set > */ > -int aa_policy_cache_remove(const char *path) > +int aa_policy_cache_remove(int dirfd, const char *path) > { > - return _aa_dirat_for_each(AT_FDCWD, path, NULL, clear_cache_cb); > + return _aa_dirat_for_each(dirfd, path, NULL, clear_cache_cb); > } > > /** > @@ -283,7 +266,7 @@ int aa_policy_cache_replace_all(aa_policy_cache > *policy_cache, > > cb_data.policy_cache = policy_cache; > cb_data.kernel_interface = kernel_interface; > - retval = _aa_dirat_for_each(AT_FDCWD, policy_cache->path, &cb_data, > + retval = _aa_dirat_for_each(policy_cache->dirfd, ".", &cb_data, > replace_all_cb); > > aa_kernel_interface_unref(kernel_interface); > diff --git a/parser/parser_main.c b/parser/parser_main.c > index dda594c..cff0813 100644 > --- a/parser/parser_main.c > +++ b/parser/parser_main.c > @@ -905,7 +905,7 @@ int main(int argc, char *argv[]) > } > > if (force_clear_cache) { > - if (aa_policy_cache_remove(cacheloc)) { > + if (aa_policy_cache_remove(AT_FDCWD, cacheloc)) { > PERROR(_("Failed to clear cache files (%s): > %s\n"), > cacheloc, strerror(errno)); > return 1; > @@ -917,8 +917,8 @@ int main(int argc, char *argv[]) > if (create_cache_dir) > pwarn(_("The --create-cache-dir option is deprecated. > Please use --write-cache.\n")); > > - retval = aa_policy_cache_new(&policy_cache, features, cacheloc, > - max_caches); > + retval = aa_policy_cache_new(&policy_cache, features, > + AT_FDCWD, cacheloc, max_caches); > if (retval) { > if (errno != ENOENT) { > PERROR(_("Failed setting up policy cache (%s): > %s\n"), > diff --git a/tests/regression/apparmor/aa_policy_cache.c > b/tests/regression/apparmor/aa_policy_cache.c > index ad3670d..d243f06 100644 > --- a/tests/regression/apparmor/aa_policy_cache.c > +++ b/tests/regression/apparmor/aa_policy_cache.c > @@ -47,7 +47,8 @@ static int test_new(const char *path, uint16_t max_caches) > aa_policy_cache *policy_cache = NULL; > int rc = 1; > > - if (aa_policy_cache_new(&policy_cache, NULL, path, max_caches)) { > + if (aa_policy_cache_new(&policy_cache, NULL, > + AT_FDCWD, path, max_caches)) { > perror("FAIL - aa_policy_cache_new"); > goto out; > } > @@ -62,7 +63,7 @@ static int test_remove(const char *path) > { > int rc = 1; > > - if (aa_policy_cache_remove(path)) { > + if (aa_policy_cache_remove(AT_FDCWD, path)) { > perror("FAIL - aa_policy_cache_remove"); > goto out; > } > @@ -98,7 +99,8 @@ static int test_replace_all(const char *path, uint16_t > max_caches) > aa_policy_cache *policy_cache = NULL; > int rc = 1; > > - if (aa_policy_cache_new(&policy_cache, NULL, path, max_caches)) { > + if (aa_policy_cache_new(&policy_cache, NULL, > + AT_FDCWD, path, max_caches)) { > perror("FAIL - aa_policy_cache_new"); > goto out; > } > -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
