[apparmor] [patch] [parser]: create missing cache directory

[Steve Beattie]

On Mon, Sep 23, 2013 at 04:13:49PM -0700, John Johansen wrote:
> This patch applies on top of the previous 2 cache patches. It does two
> things, create the cache dir if it is missing, and moves the cache clearing
> logic into the create cache routine, because if we are writing a new
> cache .features file the cache dir should be cleared out.
> 
> Signed-off-by: John Johansen <john.johansen at canonical.com>

As near as I can tell, this patch breaks the '--skip-bad-cache'
argument, but I can't really tell for sure because I'm unclear on
what that argument's purpose is.

In any event, this breakage can be seen by running the existing
tst/caching.sh script; after this commit is applied, the test:

  echo -n "Cache writing is skipped when features do not match and not cleared: "
  rm $basedir/cache/$profile
  ${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"

fails, because the cached profile gets created.

> ---
> === modified file 'parser/parser_main.c'
> --- parser/parser_main.c	2013-09-23 22:26:51 +0000
> +++ parser/parser_main.c	2013-09-23 23:09:21 +0000
> @@ -1260,20 +1260,42 @@
>  	return error;
>  }
>  
> -static int create_cache(const char *path, const char *features)
> +static int create_cache(const char *cachedir, const char *path,
> +			const char *features)
>  {
> +	struct stat stat_file;
>  	FILE * f = NULL;
>  
> +	if (cond_clear_cache && clear_cache_files(cacheloc) != 0)

If --skip-bad-cache gets passed, then cond_clear_cache gets set to 0
(false) so the goto error never occurs, instead it falls through to the
create_file target, whereupon it creates the features file (and leaves
write_cache set to 1, so it later writes out the cache file as well).

> +		goto error;
> +
> +create_file:
>  	f = fopen(path, "w");
>  	if (f) {
>  		if (fwrite(features, strlen(features), 1, f) != 1 )
> -			goto fail;
> +			goto error;
>  
>  		fclose(f);
> -	}
> -
> -	return 0;
> -fail:
> +
> +
> +		return 0;
> +	}
> +
> +error:
> +	/* does the dir exist? */
> +	if (stat(cachedir, &stat_file) == -1) {
> +		if (mkdir(cachedir, 0700) == 0)
> +			goto create_file;
> +		if (show_cache)
> +			PERROR(_("Can't create cache directory: %s\n"), cachedir);
> +	} else if (!S_ISDIR(stat_file.st_mode)) {
> +		if (show_cache)
> +			PERROR(_("File in cache directory location: %s\n"), cachedir);
> +	} else {
> +		if (show_cache)
> +			PERROR(_("Can't update cache directory: %s\n"), cachedir);
> +	}
> +
>  	if (show_cache)
>  		PERROR("Cache write disabled: cannot create %s\n", path);
>  	write_cache = 0;

This patch addresses the issue:

Signed-off-by: Steve Beattie <steve at nxnw.org>
---
 parser/parser_main.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: b/parser/parser_main.c
===================================================================
--- a/parser/parser_main.c
+++ b/parser/parser_main.c
@@ -1122,7 +1122,10 @@ static int create_cache(const char *cach
 	struct stat stat_file;
 	FILE * f = NULL;
 
-	if (cond_clear_cache && clear_cache_files(cacheloc) != 0)
+	if (!cond_clear_cache)
+		goto error;
+
+	if (clear_cache_files(cacheloc) != 0)
 		goto error;
 
 create_file:


-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20131003/b9711325/attachment.pgp>