[apparmor] [PATCH 1/6] libapparmor: Use directory file descriptor in _aa_dirat_for_each()
Tyler Hicks
tyhicks at canonical.com
Thu Mar 26 21:47:57 UTC 2015
The _aa_dirat_for_each() function used the DIR * type for its first
parameter. It then switched back and forth between the directory file
descriptors, retrieved with dirfd(), and directory streams, retrieved
with fdopendir(), when making syscalls and calling the call back
function.
This patch greatly simplifies the function by simply using directory
file descriptors. No functionality is lost since callers can still
easily use the function after calling dirfd() to retrieve the underlying
file descriptor.
Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
---
.../libapparmor/include/sys/apparmor_private.h | 5 +-
libraries/libapparmor/src/features.c | 8 +-
libraries/libapparmor/src/policy_cache.c | 12 +--
libraries/libapparmor/src/private.c | 113 +++++++--------------
parser/lib.c | 7 +-
parser/lib.h | 4 +-
parser/parser_lex.l | 5 +-
parser/parser_main.c | 9 +-
8 files changed, 61 insertions(+), 102 deletions(-)
diff --git a/libraries/libapparmor/include/sys/apparmor_private.h b/libraries/libapparmor/include/sys/apparmor_private.h
index 14055df..32b15ba 100644
--- a/libraries/libapparmor/include/sys/apparmor_private.h
+++ b/libraries/libapparmor/include/sys/apparmor_private.h
@@ -17,7 +17,6 @@
#ifndef _SYS_APPARMOR_PRIVATE_H
#define _SYS_APPARMOR_PRIVATE_H 1
-#include <dirent.h>
#include <stdio.h>
#include <sys/stat.h>
@@ -31,8 +30,8 @@ void _aa_autofclose(FILE **f);
int _aa_asprintf(char **strp, const char *fmt, ...);
-int _aa_dirat_for_each(DIR *dir, const char *name, void *data,
- int (* cb)(DIR *, const char *, struct stat *, void *));
+int _aa_dirat_for_each(int dirfd, const char *name, void *data,
+ int (* cb)(int, const char *, struct stat *, void *));
__END_DECLS
diff --git a/libraries/libapparmor/src/features.c b/libraries/libapparmor/src/features.c
index 88ded73..e7bf745 100644
--- a/libraries/libapparmor/src/features.c
+++ b/libraries/libapparmor/src/features.c
@@ -80,7 +80,7 @@ static int features_snprintf(struct features_struct *fst, const char *fmt, ...)
return 0;
}
-static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
+static int features_dir_cb(int dirfd, const char *name, struct stat *st,
void *data)
{
struct features_struct *fst = (struct features_struct *) data;
@@ -97,7 +97,7 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
int len;
int remaining = fst->size - (fst->pos - fst->buffer);
- file = openat(dirfd(dir), name, O_RDONLY);
+ file = openat(dirfd, name, O_RDONLY);
if (file == -1) {
PDEBUG("Could not open '%s'", name);
return -1;
@@ -122,7 +122,7 @@ static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
return -1;
}
} else if (S_ISDIR(st->st_mode)) {
- if (_aa_dirat_for_each(dir, name, fst, features_dir_cb))
+ if (_aa_dirat_for_each(dirfd, name, fst, features_dir_cb))
return -1;
}
@@ -137,7 +137,7 @@ static int handle_features_dir(const char *filename, char *buffer, int size,
{
struct features_struct fst = { buffer, size, pos };
- if (_aa_dirat_for_each(NULL, filename, &fst, features_dir_cb)) {
+ if (_aa_dirat_for_each(AT_FDCWD, filename, &fst, features_dir_cb)) {
PDEBUG("Failed evaluating %s\n", filename);
return -1;
}
diff --git a/libraries/libapparmor/src/policy_cache.c b/libraries/libapparmor/src/policy_cache.c
index 779c91d..22f84dd 100644
--- a/libraries/libapparmor/src/policy_cache.c
+++ b/libraries/libapparmor/src/policy_cache.c
@@ -16,8 +16,8 @@
* Ltd.
*/
-#include <dirent.h>
#include <errno.h>
+#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -36,12 +36,12 @@ struct aa_policy_cache {
char *features_path;
};
-static int clear_cache_cb(DIR *dir, const char *path, struct stat *st,
+static int clear_cache_cb(int dirfd, const char *path, struct stat *st,
void *data unused)
{
/* remove regular files */
if (S_ISREG(st->st_mode))
- return unlinkat(dirfd(dir), path, 0);
+ return unlinkat(dirfd, path, 0);
/* do nothing with other file types */
return 0;
@@ -102,7 +102,7 @@ struct replace_all_cb_data {
aa_kernel_interface *kernel_interface;
};
-static int replace_all_cb(DIR *dir unused, const char *name, struct stat *st,
+static int replace_all_cb(int dirfd unused, const char *name, struct stat *st,
void *cb_data)
{
int retval = 0;
@@ -255,7 +255,7 @@ int aa_policy_cache_make_valid(aa_policy_cache *policy_cache)
*/
int aa_policy_cache_remove(const char *path)
{
- return _aa_dirat_for_each(NULL, path, NULL, clear_cache_cb);
+ return _aa_dirat_for_each(AT_FDCWD, path, NULL, clear_cache_cb);
}
/**
@@ -285,7 +285,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(NULL, policy_cache->path, &cb_data,
+ retval = _aa_dirat_for_each(AT_FDCWD, policy_cache->path, &cb_data,
replace_all_cb);
aa_kernel_interface_unref(kernel_interface);
diff --git a/libraries/libapparmor/src/private.c b/libraries/libapparmor/src/private.c
index f078f28..f164ccb 100644
--- a/libraries/libapparmor/src/private.c
+++ b/libraries/libapparmor/src/private.c
@@ -170,25 +170,31 @@ int _aa_asprintf(char **strp, const char *fmt, ...)
return rc;
}
+static int dot_or_dot_dot_filter(const struct dirent *ent)
+{
+ if (strcmp(ent->d_name, ".") == 0 ||
+ strcmp(ent->d_name, "..") == 0)
+ return 0;
+
+ return 1;
+}
+
/**
* _aa_dirat_for_each: iterate over a directory calling cb for each entry
- * @dir: already opened directory (MAY BE NULL)
- * @name: name of the directory (MAY BE NULL)
+ * @dirfd: already opened directory
+ * @name: name of the directory (NOT NULL)
* @data: data pointer to pass to the callback fn (MAY BE NULL)
* @cb: the callback to pass entry too (NOT NULL)
*
* Iterate over the entries in a directory calling cb for each entry.
- * The directory to iterate is determined by a combination of @dir and
+ * The directory to iterate is determined by a combination of @dirfd and
* @name.
*
- * IF @name is a relative path it is determine relative to at @dir if it
- * is specified, else it the lookup is done relative to the current
- * working directory.
+ * See the openat section of the open(2) man page for details on valid @dirfd
+ * and @name combinations. This function does accept AT_FDCWD as @dirfd if
+ * @name should be considered relative to the current working directory.
*
- * If @name is not specified then @dir is used as the directory to iterate
- * over.
- *
- * It is an error if both @name and @dir are null
+ * Pass "." for @name if @dirfd is the directory to iterate over.
*
* The cb function is called with the DIR in use and the name of the
* file in that directory. If the file is to be opened it should
@@ -196,88 +202,45 @@ int _aa_asprintf(char **strp, const char *fmt, ...)
*
* Returns: 0 on success, else -1 and errno is set to the error code
*/
-int _aa_dirat_for_each(DIR *dir, const char *name, void *data,
- int (* cb)(DIR *, const char *, struct stat *, void *))
+int _aa_dirat_for_each(int dirfd, const char *name, void *data,
+ int (* cb)(int, const char *, struct stat *, void *))
{
- autofree struct dirent *dirent = NULL;
- DIR *d = NULL;
- int error;
+ autofree struct dirent **namelist = NULL;
+ autoclose int cb_dirfd = -1;
+ int i, ret;
- if (!cb || (!dir && !name)) {
+ if (!cb || !name) {
errno = EINVAL;
return -1;
}
- if (dir && (!name || *name != '/')) {
- dirent = (struct dirent *)
- malloc(offsetof(struct dirent, d_name) +
- fpathconf(dirfd(dir), _PC_NAME_MAX) + 1);
- } else {
- dirent = (struct dirent *)
- malloc(offsetof(struct dirent, d_name) +
- pathconf(name, _PC_NAME_MAX) + 1);
- }
- if (!dirent) {
- errno = ENOMEM;
- PDEBUG("could not alloc dirent: %m\n");
+ cb_dirfd = openat(dirfd, name, O_RDONLY | O_CLOEXEC | O_DIRECTORY);
+ if (cb_dirfd == -1) {
+ PDEBUG("could not open directory '%s': %m\n", name);
return -1;
}
- if (name) {
- if (dir && *name != '/') {
- int fd = openat(dirfd(dir), name, O_RDONLY);
- if (fd == -1)
- goto fail;
- d = fdopendir(fd);
- } else {
- d = opendir(name);
- }
- PDEBUG("Open dir '%s': %s\n", name, d ? "succeeded" : "failed");
- if (!(d))
- goto fail;
- } else { /* dir && !name */
- PDEBUG("Recieved cache directory\n");
- d = dir;
+ ret = scandirat(cb_dirfd, ".", &namelist, dot_or_dot_dot_filter, NULL);
+ if (ret == -1) {
+ PDEBUG("scandirat of directory '%s' failed: %m\n", name);
+ return -1;
}
- for (;;) {
- struct dirent *ent;
+ for (i = 0; i < ret; i++) {
struct stat my_stat;
- error = readdir_r(d, dirent, &ent);
- if (error) {
- errno = error; /* readdir_r directly returns an errno */
- PDEBUG("readdir_r failed: %m\n");
- goto fail;
- } else if (!ent) {
- break;
- }
-
- if (strcmp(ent->d_name, ".") == 0 ||
- strcmp(ent->d_name, "..") == 0)
- continue;
-
- if (fstatat(dirfd(d), ent->d_name, &my_stat, 0)) {
- PDEBUG("stat failed for '%s': %m\n", name);
- goto fail;
+ if (fstatat(cb_dirfd, namelist[i]->d_name, &my_stat, 0)) {
+ PDEBUG("stat failed for '%s': %m\n",
+ namelist[i]->d_name);
+ return -1;
}
- if (cb(d, ent->d_name, &my_stat, data)) {
- PDEBUG("dir_for_each callback failed\n");
- goto fail;
+ if (cb(cb_dirfd, namelist[i]->d_name, &my_stat, data)) {
+ PDEBUG("dir_for_each callback failed for '%s'\n",
+ namelist[i]->d_name);
+ return -1;
}
}
- if (d != dir)
- closedir(d);
-
return 0;
-
-fail:
- error = errno;
- if (d && d != dir)
- closedir(d);
- errno = error;
-
- return -1;
}
diff --git a/parser/lib.c b/parser/lib.c
index 6aae670..11c2210 100644
--- a/parser/lib.c
+++ b/parser/lib.c
@@ -16,7 +16,6 @@
* Ltd.
*/
-#include <dirent.h>
#include <string.h>
#include <sys/stat.h>
@@ -29,10 +28,10 @@
#include "lib.h"
#include "parser.h"
-int dirat_for_each(DIR *dir, const char *name, void *data,
- int (* cb)(DIR *, const char *, struct stat *, void *))
+int dirat_for_each(int dirfd, const char *name, void *data,
+ int (* cb)(int, const char *, struct stat *, void *))
{
- int retval = _aa_dirat_for_each(dir, name, data, cb);
+ int retval = _aa_dirat_for_each(dirfd, name, data, cb);
if (retval)
PDEBUG("dirat_for_each failed: %m\n");
diff --git a/parser/lib.h b/parser/lib.h
index a980a5a..e9a8567 100644
--- a/parser/lib.h
+++ b/parser/lib.h
@@ -9,8 +9,8 @@
#define asprintf _aa_asprintf
-int dirat_for_each(DIR *dir, const char *name, void *data,
- int (* cb)(DIR *, const char *, struct stat *, void *));
+int dirat_for_each(int dirfd, const char *name, void *data,
+ int (* cb)(int, const char *, struct stat *, void *));
int isodigit(char c);
long strntol(const char *str, const char **endptr, int base, long maxval,
diff --git a/parser/parser_lex.l b/parser/parser_lex.l
index 0456843..ef8add0 100644
--- a/parser/parser_lex.l
+++ b/parser/parser_lex.l
@@ -32,7 +32,6 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
-#include <dirent.h>
#include <unordered_map>
#include <string>
@@ -120,7 +119,7 @@ struct cb_struct {
const char *filename;
};
-static int include_dir_cb(DIR *dir unused, const char *name, struct stat *st,
+static int include_dir_cb(int dirfd unused, const char *name, struct stat *st,
void *data)
{
struct cb_struct *d = (struct cb_struct *) data;
@@ -179,7 +178,7 @@ void include_filename(char *filename, int search)
struct cb_struct data = { fullpath, filename };
fclose(include_file);
include_file = NULL;
- if (dirat_for_each(NULL, fullpath, &data, include_dir_cb)) {
+ if (dirat_for_each(AT_FDCWD, fullpath, &data, include_dir_cb)) {
yyerror(_("Could not process include directory"
" '%s' in '%s'"), fullpath, filename);;
}
diff --git a/parser/parser_main.c b/parser/parser_main.c
index 1dc3088..1069e9d 100644
--- a/parser/parser_main.c
+++ b/parser/parser_main.c
@@ -28,7 +28,6 @@
#include <getopt.h>
#include <errno.h>
#include <fcntl.h>
-#include <dirent.h>
/* enable the following line to get voluminous debug info */
/* #define DEBUG */
@@ -811,7 +810,7 @@ struct dir_cb_data {
};
/* data - pointer to a dir_cb_data */
-static int profile_dir_cb(DIR *dir unused, const char *name, struct stat *st,
+static int profile_dir_cb(int dirfd unused, const char *name, struct stat *st,
void *data)
{
int rc = 0;
@@ -828,7 +827,7 @@ static int profile_dir_cb(DIR *dir unused, const char *name, struct stat *st,
}
/* data - pointer to a dir_cb_data */
-static int binary_dir_cb(DIR *dir unused, const char *name, struct stat *st,
+static int binary_dir_cb(int dirfd unused, const char *name, struct stat *st,
void *data)
{
int rc = 0;
@@ -965,14 +964,14 @@ int main(int argc, char *argv[])
}
if (profilename && S_ISDIR(stat_file.st_mode)) {
- int (*cb)(DIR *dir, const char *name, struct stat *st,
+ int (*cb)(int dirfd, const char *name, struct stat *st,
void *data);
struct dir_cb_data cb_data;
cb_data.dirname = profilename;
cb_data.cachedir = cacheloc;
cb = binary_input ? binary_dir_cb : profile_dir_cb;
- if ((retval = dirat_for_each(NULL, profilename,
+ if ((retval = dirat_for_each(AT_FDCWD, profilename,
&cb_data, cb))) {
PDEBUG("Failed loading profiles from %s\n",
profilename);
--
2.1.4
More information about the AppArmor
mailing list