[apparmor] [PATCH v2 11/14] libapparmor: Migrate aa_policy_cache API to openat() style
John Johansen
john.johansen at canonical.com
Fri May 29 11:30:46 UTC 2015
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 <tyhicks at canonical.com>
Acked-by: John Johansen <john.johansen at canonical.com>
> ---
> 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;
> }
>
More information about the AppArmor
mailing list