apparmor: abstract out the directory walking routine

The apparmor_parser has 3 different directory walking routines. Abstract
them out and use a single common routine.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>

parser/Makefile

@@ -76,8 +76,8 @@ EXTRA_CFLAGS+=-DSUBDOMAIN_CONFDIR=\"${CONFDIR}\"
 SRCS = parser_common.c parser_include.c parser_interface.c parser_lex.c \
        parser_main.c parser_misc.c parser_merge.c parser_symtab.c \
        parser_yacc.c parser_regex.c parser_variable.c parser_policy.c \
-       parser_alias.c mount.c
+       parser_alias.c mount.c lib.c
-HDRS = parser.h parser_include.h immunix.h mount.h
+HDRS = parser.h parser_include.h immunix.h mount.h lib.h
 TOOLS = apparmor_parser
 
 OBJECTS = $(SRCS:.c=.o)
@@ -204,6 +204,9 @@ parser_common.o: parser_common.c parser.h
 mount.o: mount.c mount.h parser.h immunix.h
 	$(CC) $(EXTRA_CFLAGS) -c -o $@ $<
 
+lib.o: lib.c lib.h parser.h
+	$(CC) $(EXTRA_CFLAGS) -c -o $@ $<
+
 parser_version.h: Makefile
 	@echo \#define PARSER_VERSION \"$(VERSION)\" > .ver
 	@mv -f .ver $@

parser/lib.c

@@ -0,0 +1,135 @@
+/*
+ *   Copyright (c) 2012
+ *   Canonical Ltd. (All rights reserved)
+ *
+ *   This program is free software; you can redistribute it and/or
+ *   modify it under the terms of version 2 of the GNU General Public
+ *   License published by the Free Software Foundation.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program; if not, contact Novell, Inc. or Canonical,
+ *   Ltd.
+ */
+
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <unistd.h>
+#define _(s) gettext(s)
+
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#include "parser.h"
+
+/**
+ * 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)
+ * @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
+ * @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.
+ *
+ * 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
+ *
+ * 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
+ * use the openat, fstatat, and related fns.
+ *
+ * Returns: 0 on success, else -1 and errno is set to the error code
+ */
+int dirat_for_each(DIR *dir, const char *name, void *data,
+		   int (* cb)(DIR *, const char *, struct stat *, void *))
+{
+	struct dirent *dirent = NULL, *ent;
+	DIR *d = NULL;
+	int error = 0;
+
+	if (!cb || (!dir && !name)) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	if (dir && (!name || *name != '/')) {
+		dirent = malloc(offsetof(struct dirent, d_name) +
+				fpathconf(dirfd(dir), _PC_NAME_MAX) + 1);
+	} else {
+		dirent = malloc(offsetof(struct dirent, d_name) +
+				pathconf(name, _PC_NAME_MAX) + 1);
+	}
+	if (!dirent) {
+		PDEBUG("could not alloc dirent");
+		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;
+	}
+
+	for (error = readdir_r(d, dirent, &ent);
+	     error == 0 && ent != NULL;
+	     error = readdir_r(d, dirent, &ent)) {
+		struct stat my_stat;
+
+		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'", name);
+			goto fail;
+		}
+
+		if (cb(d, ent->d_name, &my_stat, data)) {
+			PDEBUG("dir_for_each callback failed\n");
+			goto fail;
+		}
+	}
+
+	if (d != dir)
+		closedir(d);
+	free(dirent);
+
+	return error;
+
+fail:
+	error = errno;
+	if (d && d != dir)
+		closedir(d);
+	free(dirent);
+	errno = error;
+
+	return -1;
+}

parser/lib.h

@@ -0,0 +1,7 @@
+#ifndef __AA_LIB_H_
+#define __AA_LIB_H_
+
+int dirat_for_each(DIR *dir, const char *name, void *data,
+		   int (* cb)(DIR *, const char *, struct stat *, void *));
+
+#endif /* __AA_LIB_H_ */

parser/parser_lex.l

@@ -39,6 +39,7 @@
 #include "parser.h"
 #include "parser_include.h"
 #include "parser_yacc.h"
+#include "lib.h"
 
 #ifdef PDEBUG
 #undef PDEBUG
@@ -77,6 +78,61 @@ struct ignored_suffix_t ignored_suffixes[] = {
 	{ NULL, 0, 0 }
 };
 
+static int is_blacklisted(const char *name, const char *path)
+{
+	int name_len;
+	struct ignored_suffix_t *suffix;
+	name_len = strlen(name);
+	/* skip blacklisted suffixes */
+	for (suffix = ignored_suffixes; suffix->text; suffix++) {
+		char *found;
+		if ( (found = strstr(name, suffix->text)) &&
+		     found - name + suffix->len == name_len ) {
+			if (!suffix->silent)
+				PERROR("Ignoring: '%s'\n", path);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+struct cb_struct {
+	const char *fullpath;
+	const char *filename;
+};
+
+static int include_dir_cb(__unused DIR *dir, const char *name, struct stat *st,
+			  void *data)
+{
+	struct cb_struct *d = (struct cb_struct *) data;
+
+	char *path;
+
+	/* skip dotfiles silently. */
+	if (*name == '.')
+		return 0;
+
+	if (asprintf(&path, "%s/%s", d->fullpath, name) < 0)
+		yyerror("Out of memory");
+
+	if (is_blacklisted(name, path))
+		return 0;
+
+	if (S_ISREG(st->st_mode)) {
+		if (!(yyin = fopen(path,"r")))
+			yyerror(_("Could not open '%s' in '%s'"), path, d->filename);
+		PDEBUG("Opened include \"%s\" in \"%s\"\n", path, d->filename);
+		update_mru_tstamp(yyin);
+		push_include_stack(path);
+		yypush_buffer_state(yy_create_buffer(yyin, YY_BUF_SIZE));
+	}
+
+	free(path);
+
+	return 0;
+}
+
 void include_filename(char *filename, int search)
 {
 	FILE *include_file = NULL;
@@ -107,60 +163,15 @@ void include_filename(char *filename, int search)
 		PDEBUG("Opened include \"%s\"\n", fullpath);
 		push_include_stack(fullpath);
 		yypush_buffer_state(yy_create_buffer( yyin, YY_BUF_SIZE ));
-        }
+        } else if (S_ISDIR(my_stat.st_mode)) {
-
+		struct cb_struct data = { fullpath, filename };
-        if (S_ISDIR(my_stat.st_mode)) {
-                DIR *dir = NULL;
-		char *dirent_path = NULL;
-                struct dirent *dirent;
-
-		PDEBUG("Opened include directory \"%s\"\n", fullpath);
-                if (!(dir = opendir(fullpath)))
-			yyerror(_("opendir failed '%s'"), fullpath);
 		fclose(include_file);
 		include_file = NULL;
-
+		if (dirat_for_each(NULL, fullpath, &data, include_dir_cb)) {
-                while ((dirent = readdir(dir)) != NULL) {
+			yyerror(_("Could not process include directory"
-			int name_len;
+				  " '%s' in '%s'"), fullpath, filename);;
-			struct ignored_suffix_t *suffix;
-                        /* skip dotfiles silently. */
-                        if (dirent->d_name[0] == '.')
-                                continue;
-
-			if (dirent_path)
-				free(dirent_path);
-                        if (asprintf(&dirent_path, "%s/%s", fullpath, dirent->d_name) < 0)
-				yyerror("Out of memory");
-
-			name_len = strlen(dirent->d_name);
-			/* skip blacklisted suffixes */
-			for (suffix = ignored_suffixes; suffix->text; suffix++) {
-				char *found;
-				if ( (found = strstr(dirent->d_name, suffix->text)) &&
-                                     found - dirent->d_name + suffix->len == name_len ) {
-					name_len = 0;
-					if (!suffix->silent)
-						PERROR("Ignoring: '%s'\n", dirent_path);
-					break;
 		}
 	}
-			if (!name_len) continue;
-
-                        if (stat(dirent_path, &my_stat))
-				yyerror(_("stat failed for '%s'"), dirent_path);
-			if (S_ISREG(my_stat.st_mode)) {
-				if (!(yyin = fopen(dirent_path,"r")))
-					yyerror(_("Could not open '%s' in '%s'"), dirent_path, filename);
-				PDEBUG("Opened include \"%s\" in \"%s\"\n", dirent_path, filename);
-				update_mru_tstamp(yyin);
-				push_include_stack(dirent_path);
-				yypush_buffer_state(yy_create_buffer(yyin, YY_BUF_SIZE));
-			}
-		}
-		if (dirent_path)
-			free(dirent_path);
-		closedir(dir);
-	}
 
 	if (fullpath)
 		free(fullpath);

parser/parser_main.c

@@ -43,6 +43,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
+#include "lib.h"
 #include "parser.h"
 #include "parser_version.h"
 #include "parser_include.h"
@@ -707,89 +708,70 @@ char *snprintf_buffer(char *buf, char *pos, ssize_t size, const char *fmt, ...)
 	return pos + i;
 }
 
-static char *handle_features_dir(const char *filename, char **buffer, int size,
+struct features_struct {
-				 char *pos)
+	char **buffer;
+	int size;
+	char *pos;
+};
+
+static int features_dir_cb(DIR *dir, const char *name, struct stat *st,
+			   void *data)
 {
-	DIR *dir = NULL;
+	struct features_struct *fst = (struct features_struct *) data;
-	char *dirent_path = NULL;
-	struct dirent *dirent;
-	struct stat my_stat;
-	int len;
 
-	PDEBUG("Opened features directory \"%s\"\n", filename);
+	/* skip dot files and files with no name */
-	if (!(dir = opendir(filename))) {
+	if (*name == '.' || !strlen(name))
-		PDEBUG("opendir failed '%s'", filename);
+		return 0;
-		exit(1);
+
+	fst->pos = snprintf_buffer(*fst->buffer, fst->pos, fst->size, "%s {", name);
+
+	if (S_ISREG(st->st_mode)) {
+		int len, file;
+		int remaining = fst->size - (fst->pos - *fst->buffer);
+		if (!(file = openat(dirfd(dir), name, O_RDONLY))) {
+			PDEBUG("Could not open '%s'", name);
+			return -1;
 		}
-
+		PDEBUG("Opened features \"%s\"\n", name);
-	while ((dirent = readdir(dir)) != NULL) {
+		if (st->st_size > remaining) {
-		int name_len;
+			PDEBUG("Feature buffer full.");
-		/* skip dotfiles silently. */
+			return -1;
-		if (dirent->d_name[0] == '.')
-			continue;
-
-		if (dirent_path)
-			free(dirent_path);
-		if (asprintf(&dirent_path, "%s/%s", filename, dirent->d_name) < 0)
-		{
-			PERROR(_("Memory allocation error."));
-			exit(1);
-		}
-
-		name_len = strlen(dirent->d_name);
-		if (!name_len)
-			continue;
-
-		if (stat(dirent_path, &my_stat)) {
-			PERROR(_("stat failed for '%s'"), dirent_path);
-			exit(1);
-		}
-
-		pos = snprintf_buffer(*buffer, pos, size, "%s {", dirent->d_name);
-		if (S_ISREG(my_stat.st_mode)) {
-			int file;
-			int remaining = size - (pos - *buffer);
-			if (!(file = open(dirent_path, O_RDONLY))) {
-				PDEBUG("Could not open '%s' in '%s'", dirent_path, filename);
-				exit(1);
-				break;
-			}
-			PDEBUG("Opened features \"%s\" in \"%s\"\n", dirent_path, filename);
-			if (my_stat.st_size > remaining) {
-				PERROR(_("Feature buffer full."));
-				exit(1);
 		}
 
 		do {
-				len = read(file, pos, remaining);
+			len = read(file, fst->pos, remaining);
 			if (len > 0) {
 				remaining -= len;
-					pos += len;
+				fst->pos += len;
-					*pos = 0;
+				*fst->pos = 0;
 			}
 		} while (len > 0);
 		if (len < 0) {
-				PDEBUG("Error reading feature file '%s'\n",
+			PDEBUG("Error reading feature file '%s'\n", name);
-				       dirent_path);
+			return -1;
-				exit(1);
 		}
 		close(file);
-
+	} else if (S_ISDIR(st->st_mode)) {
-		} else if (S_ISDIR(my_stat.st_mode)) {
+		if (dirat_for_each(dir, name, fst, features_dir_cb))
-			pos = handle_features_dir(dirent_path, buffer, size,
+			return -1;
-						  pos);
-			if (!pos)
-				break;
-
 	}
 
-		pos = snprintf_buffer(*buffer, pos, size, "}\n");
+	fst->pos = snprintf_buffer(*fst->buffer, fst->pos, fst->size, "}\n");
-	}
-	if (dirent_path)
-		free(dirent_path);
-	closedir(dir);
 
-	return pos;
+	return 0;
+}
+
+static char *handle_features_dir(const char *filename, char **buffer, int size,
+				 char *pos)
+{
+	struct features_struct fst = { buffer, size, pos };
+
+	if (dirat_for_each(NULL, filename, &fst, features_dir_cb)) {
+		PDEBUG("Failed evaluating .features\n");
+		exit(1);
+	}
+
+	return fst.pos;
 }
 
 /* match_string == NULL --> no match_string available
@@ -1180,77 +1162,12 @@ out:
 	return retval;
 }
 
-static int dir_for_each(const char *dname,
+static int clear_cache_cb(DIR *dir, const char *path, struct stat *st,
-			int (* callback)(const char *, struct dirent *,
+			  __unused void *data)
-					 struct stat *)) {
-	struct dirent *dirent, *ent;
-	char *path = NULL;
-	DIR *dir = NULL;
-	int error;
-
-	dirent = malloc(offsetof(struct dirent, d_name) +
-			pathconf(dname, _PC_NAME_MAX) + 1);
-	if (!dirent) {
-		PDEBUG(_("could not alloc dirent"));
-		return -1;
-	}
-
-	PDEBUG("Opened cache directory \"%s\"\n", dname);
-	if (!(dir = opendir(dname))) {
-		free(dirent);
-		PDEBUG(_("opendir failed '%s'"), dname);
-		return -1;
-	}
-
-	for (error = readdir_r(dir, dirent, &ent);
-	     error == 0 && ent != NULL;
-	     error = readdir_r(dir, dirent, &ent)) {
-		struct stat my_stat;
-
-		if (strcmp(dirent->d_name, ".") == 0 ||
-		    strcmp(dirent->d_name, "..") == 0)
-			continue;
-
-		if (asprintf(&path, "%s/%s", dname, dirent->d_name) < 0)
-		{
-			PDEBUG(_("Memory allocation error."));
-			goto fail;
-		}
-	
-		if (stat(path, &my_stat)) {
-			PDEBUG(_("stat failed for '%s'"), path);
-			goto fail;
-		}
-
-		if (callback(path, dirent, &my_stat)) {
-			PDEBUG(_("dir_for_each callback failed\n"));
-			goto fail;
-		}
-
-		free(path);
-		path = NULL;
-	}
-
-	free(dirent);
-	closedir(dir);
-	return error;
-
-fail:
-	error = errno;
-	free(dirent);
-	free(path);
-	closedir(dir);
-	errno = error;
-
-	return -1;
-}
-
-static int clear_cache_cb(const char *path, __unused struct dirent *dirent,
-			  struct stat *ent_stat)
 {
 	/* remove regular files */
-	if (S_ISREG(ent_stat->st_mode))
+	if (S_ISREG(st->st_mode))
-		return unlink(path);
+		return unlinkat(dirfd(dir), path, 0);
 
 	/* do nothing with other file types */
 	return 0;
@@ -1266,7 +1183,7 @@ static int clear_cache_files(const char *path)
 		exit(1);
 	}
 
-	error = dir_for_each(cache, clear_cache_cb);
+	error = dirat_for_each(NULL, cache, NULL, clear_cache_cb);
 
 	free(cache);
 

parser/tst/caching.sh

@@ -122,7 +122,7 @@ echo -n "monkey" > $basedir/cache/.features
 echo -n "monkey" > $basedir/cache/$profile
 echo -n "monkey" > $basedir/cache/monkey
 echo -n "Cache purge remove profiles unconditionally: "
-../apparmor_parser $ARGS -v --purge-cache -r $basedir/$profile || { echo "Cache clear setup FAIL"; exit 1; }
+../apparmor_parser $ARGS -v --purge-cache -r $basedir/$profile || { echo "Cache purge setup FAIL"; exit 1; }
 [ -f $basedir/cache/.features ] && { echo "FAIL"; exit 1; }
 [ -f $basedir/cache/$profile ] && { echo "FAIL"; exit 1; }
 [ -f $basedir/cache/monkey ] && { echo "FAIL"; exit 1; }