[apparmor] Cache update broken

Seth Arnold seth.arnold at gmail.com
Tue Aug 7 22:26:17 UTC 2012


Excellent, it feels much improved despite breaking Christian's Rules. :)

Could you add another monkey test for --purge-cache?

The short option -W leaked into the manpage update.

Thanks John.
-----Original Message-----
From: John Johansen <john.johansen at canonical.com>
Sender: apparmor-bounces at lists.ubuntu.com
Date: Tue, 07 Aug 2012 15:14:21 
To: Christian Boltz<apparmor at cboltz.de>
Cc: <apparmor at lists.ubuntu.com>
Subject: Re: [apparmor] Cache update broken

On 08/04/2012 06:53 AM, Christian Boltz wrote:
> Hello,
> 
okay attached is v2 of the patch, its changes are:

* makes cache clearing the default behavior if --write-cache is enabled
* changes --no-clear-cache to --skip-bad-cache (really the same thing but
  hopefully a better name?)
* adds a --purge-cache debug option to force the cache to be cleared
  regardless of state
* adds documentation to the apparmor_parser man page

---

apparmor: add clearing the profile cache when inconsistent

Add the ability to clear out the binary profile cache. This removes the
need to have a separate script to handle the logic of checking and
removing the cache if it is out of date.

The parser already does all the checking to determine cache validity
so it makes sense to allow the parser to clear out inconsistent cache
when it has been instructed to update the cache.

Signed-off-by: John Johnansen <john.johansen at canonical.com>

---

=== modified file 'parser/apparmor_parser.pod'
--- parser/apparmor_parser.pod	2012-02-24 12:21:59 +0000
+++ parser/apparmor_parser.pod	2012-08-07 22:06:34 +0000
@@ -138,6 +138,15 @@
 is running with "--replace", it may make sense to also use
 "--skip-read-cache" with the "--write-cache" option.
 
+=item -W, --purge-cache
+
+Unconditionally clear out cached profiles.
+
+=item -W, --skip-bad-cache
+
+Skip updating the cache if it contains cached profiles in a bad or
+inconsistant state
+
 =item -L, --cache-loc
 
 Set the location of the cache directory.  If not specified the cache location

=== modified file 'parser/parser_main.c'
--- parser/parser_main.c	2012-07-17 23:00:53 +0000
+++ parser/parser_main.c	2012-08-07 22:00:54 +0000
@@ -24,6 +24,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <stdarg.h>
+#include <stddef.h>
 #include <getopt.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -71,6 +72,8 @@
 int skip_cache = 0;
 int skip_read_cache = 0;
 int write_cache = 0;
+int cond_clear_cache = 1;		/* only applies if write is set */
+int force_clear_cache = 0;		/* force clearing regargless of state */
 int preprocess_only = 0;
 int skip_mode_force = 0;
 struct timespec mru_tstamp;
@@ -109,6 +112,8 @@
 	{"skip-read-cache",	0, 0, 'T'},
 	{"write-cache",		0, 0, 'W'},
 	{"show-cache",		0, 0, 'k'},
+	{"skip-bad-cache",	0, 0, 129},	/* no short option */
+	{"purge-cache",		0, 0, 130},	/* no short option */
 	{"cache-loc",		1, 0, 'L'},
 	{"debug",		0, 0, 'd'},
 	{"dump",		1, 0, 'D'},
@@ -151,6 +156,8 @@
 	       "-K, --skip-cache	Do not attempt to load or save cached profiles\n"
 	       "-T, --skip-read-cache	Do not attempt to load cached profiles\n"
 	       "-W, --write-cache	Save cached profile (force with -T)\n"
+	       "    --skip-bad-cache	Don't clear cache if out of sync\n"
+	       "    --purge-cache	Clear cache regardless of its state\n"
 	       "-L, --cache-loc n	Set the location of the profile cache\n"
 	       "-q, --quiet		Don't emit warnings\n"
 	       "-v, --verbose		Show profile names as they load\n"
@@ -527,6 +534,12 @@
 	case 'T':
 		skip_read_cache = 1;
 		break;
+	case 129:
+		cond_clear_cache = 0;
+		break;
+	case 130:
+		force_clear_cache = 1;
+		break;
 	case 'L':
 		cacheloc = strdup(optarg);
 		break;
@@ -1165,11 +1178,128 @@
 	return retval;
 }
 
+static int dir_for_each(const char *dname,
+			int (* callback)(const char *, struct dirent *,
+					 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))
+		return unlink(path);
+
+	/* do nothing with other file types */
+	return 0;
+}
+
+static int clear_cache_files(const char *path)
+{
+	char *cache;
+	int error;
+
+	if (asprintf(&cache, "%s/cache", path) == -1) {
+		perror("asprintf");
+		exit(1);
+	}
+
+	error = dir_for_each(cache, clear_cache_cb);
+
+	free(cache);
+
+	return error;
+}
+
+static int create_cache(const char *path, const char *features)
+{
+	FILE * f = NULL;
+
+	f = fopen(path, "w");
+	if (f) {
+		if (fwrite(features, strlen(features), 1, f) != 1 )
+			goto fail;
+
+		fclose(f);
+	}
+
+	return 0;
+fail:
+	if (show_cache)
+		PERROR("Cache write disabled: cannot create %s\n", path);
+	write_cache = 0;
+
+	return -1;
+}
+
 static void setup_flags(void)
 {
 	char *cache_features_path = NULL;
 	char *cache_flags = NULL;
 
+	if (force_clear_cache)
+		clear_cache_files(basedir);
+
 	/* Get the match string to determine type of regex support needed */
 	get_match_string();
 	/* Get kernel features string */
@@ -1203,30 +1333,23 @@
 	get_flags_string(&cache_flags, cache_features_path);
 	if (cache_flags) {
 		if (strcmp(flags_string, cache_flags) != 0) {
-			if (show_cache) PERROR("Cache read/write disabled: %s does not match %s\n", FLAGS_FILE, cache_features_path);
-			write_cache = 0;
-			skip_read_cache = 1;
+			if (write_cache && cond_clear_cache) {
+				if (clear_cache_files(basedir) ||
+				    create_cache(cache_features_path,
+						 flags_string)) {
+					skip_read_cache = 1;
+				}
+			} else {
+				if (show_cache)
+					PERROR("Cache read/write disabled: %s does not match %s\n", FLAGS_FILE, cache_features_path);
+				write_cache = 0;
+				skip_read_cache = 1;
+			}
 		}
 		free(cache_flags);
 		cache_flags = NULL;
-	}
-	else if (write_cache) {
-		FILE * f = NULL;
-		int failure = 0;
-
-		f = fopen(cache_features_path, "w");
-		if (!f) failure = 1;
-		else {
-			if (fwrite(flags_string, strlen(flags_string), 1, f) != 1 ) {
-				failure = 1;
-			}
-			if (fclose(f) != 0) failure = 1;
-		}
-
-		if (failure) {
-			if (show_cache) PERROR("Cache write disabled: cannot write to %s\n", cache_features_path);
-			write_cache = 0;
-		}
+	} else if (write_cache) {
+		create_cache(cache_features_path, flags_string);
 	}
 
 	free(cache_features_path);

=== modified file 'parser/tst/caching.sh'
--- parser/tst/caching.sh	2012-03-09 12:25:03 +0000
+++ parser/tst/caching.sh	2012-08-07 22:03:05 +0000
@@ -93,12 +93,29 @@
 ../apparmor_parser $ARGS -v -r $basedir/$profile | grep -q 'Replacement succeeded for' || { echo "FAIL"; exit 1; }
 echo "ok"
 
-echo -n "Cache writing is skipped when features do not match cache: "
+echo -n "Cache writing is skipped when features do not match and not cleared: "
 rm $basedir/cache/$profile
-../apparmor_parser $ARGS -v --write-cache -r $basedir/$profile | grep -q 'Replacement succeeded for' || { echo "FAIL"; exit 1; }
+../apparmor_parser $ARGS -v --write-cache --skip-bad-cache -r $basedir/$profile | grep -q 'Replacement succeeded for' || { echo "FAIL"; exit 1; }
 [ -f $basedir/cache/$profile ] && echo "FAIL ($basedir/cache/$profile exists)" && exit 1
 echo "ok"
 
+rm -f $basedir/cache/.features || true
+rm -f $basedir/cache/$profile || true
+echo -n "monkey" > $basedir/cache/.features
+echo -n "monkey" > $basedir/cache/$profile
+echo -n "monkey" > $basedir/cache/monkey
+../apparmor_parser $ARGS -v --write-cache -r $basedir/$profile  || { echo "Cache clear setup FAIL"; exit 1; }
+echo -n "Cache clear updates features: "
+echo -n "monkey" | diff -q $basedir/cache/.features - | grep -q 'differ' || { echo "FAIL"; exit 1; }
+echo "ok"
+echo -n "Cache clear writes updated profile: "
+echo -n "monkey" | diff -q $basedir/cache/$profile - | grep -q 'differ' || { echo "FAIL"; exit 1; }
+echo "ok"
+echo -n "Cache clear cleans out all files: "
+[ -f $basedir/cache/monkey ] && { echo "FAIL"; exit 1; }
+echo "ok"
+rm -f $basedir/cache/monkey
+
 echo -n "Profiles are cached when requested (again): "
 rm -f $basedir/cache/.features || true
 rm -f $basedir/cache/$profile || true




-- 
AppArmor mailing list
AppArmor at lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor


More information about the AppArmor mailing list