On Tue, 2012-08-07 at 15:14 -0700, John Johansen wrote: > 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 >
This seems reasonable to me with my Ubuntu hat on. The two places that
we use '--write-cache' are in our 'load_configured_profiles()' and in
'recache_profiles()' from /lib/apparmor/functions. The former implements
the removal of an outdated cache and the latter is essentially
--purge-cache (I also think that --purge-cache be default should clear
out all the profiles).
As such, Ubuntu could probably clean up the startup scripts by doing
this, but we aren't immediately forced to either.
> ---
>
> 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.johan...@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
>
>
>
>
--
Jamie Strandboge | http://www.canonical.com
signature.asc
Description: This is a digitally signed message part
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
