[apparmor] [PATCH 6/6] libapparmor: Migrate aa_policy_cache API to openat() style
Tyler Hicks
tyhicks at canonical.com
Thu Mar 26 21:48:02 UTC 2015
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>
---
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 | 10 +--
4 files changed, 48 insertions(+), 65 deletions(-)
diff --git a/libraries/libapparmor/include/sys/apparmor.h b/libraries/libapparmor/include/sys/apparmor.h
index 435fb09..d37ba6e 100644
--- a/libraries/libapparmor/include/sys/apparmor.h
+++ b/libraries/libapparmor/include/sys/apparmor.h
@@ -142,14 +142,14 @@ 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,
- bool create);
+ aa_features *kernel_features,
+ int dirfd, const char *path, bool create);
aa_policy_cache *aa_policy_cache_ref(aa_policy_cache *policy_cache);
void aa_policy_cache_unref(aa_policy_cache *policy_cache);
bool aa_policy_cache_is_valid(aa_policy_cache *policy_cache);
int aa_policy_cache_make_valid(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 a5eff24..b3126d5 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,44 +50,23 @@ 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,
aa_features *kernel_features, bool create)
{
- 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;
@@ -109,17 +89,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;
@@ -131,6 +105,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
* @create: true if the cache should be created if it doesn't already exist
*
@@ -138,8 +113,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,
- bool create)
+ aa_features *kernel_features,
+ int dirfd, const char *path, bool create)
{
aa_policy_cache *pc;
@@ -155,19 +130,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;
}
@@ -216,8 +198,8 @@ 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);
}
}
@@ -249,13 +231,14 @@ int aa_policy_cache_make_valid(aa_policy_cache *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);
}
/**
@@ -285,7 +268,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 555620d..7ec1d95 100644
--- a/parser/parser_main.c
+++ b/parser/parser_main.c
@@ -903,7 +903,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;
@@ -915,8 +915,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,
- write_cache);
+ retval = aa_policy_cache_new(&policy_cache, features,
+ AT_FDCWD, cacheloc, write_cache);
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 b221c98..fafe758 100644
--- a/tests/regression/apparmor/aa_policy_cache.c
+++ b/tests/regression/apparmor/aa_policy_cache.c
@@ -50,7 +50,7 @@ static int test_make_valid(const char *path)
aa_policy_cache *policy_cache = NULL;
int rc = 1;
- if (aa_policy_cache_new(&policy_cache, NULL, path, false)) {
+ if (aa_policy_cache_new(&policy_cache, NULL, AT_FDCWD, path, false)) {
perror("FAIL - aa_policy_cache_new");
goto out;
}
@@ -71,7 +71,7 @@ static int test_is_valid(const char *path)
aa_policy_cache *policy_cache = NULL;
int rc = 1;
- if (aa_policy_cache_new(&policy_cache, NULL, path, false)) {
+ if (aa_policy_cache_new(&policy_cache, NULL, AT_FDCWD, path, false)) {
perror("FAIL - aa_policy_cache_new");
goto out;
}
@@ -93,7 +93,7 @@ static int test_new(const char *path, bool create)
aa_policy_cache *policy_cache = NULL;
int rc = 1;
- if (aa_policy_cache_new(&policy_cache, NULL, path, create)) {
+ if (aa_policy_cache_new(&policy_cache, NULL, AT_FDCWD, path, create)) {
perror("FAIL - aa_policy_cache_new");
goto out;
}
@@ -108,7 +108,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;
}
@@ -144,7 +144,7 @@ static int test_replace_all(const char *path)
aa_policy_cache *policy_cache = NULL;
int rc = 1;
- if (aa_policy_cache_new(&policy_cache, NULL, path, false)) {
+ if (aa_policy_cache_new(&policy_cache, NULL, AT_FDCWD, path, false)) {
perror("FAIL - aa_policy_cache_new");
goto out;
}
--
2.1.4
More information about the AppArmor
mailing list