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

[John Johansen]

On 10/03/2013 09:20 AM, Steve Beattie wrote:
> 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.
> 
its whether the cache gets cleared when an error is detected, so
the inverse of the cond_clear_cache variable (don't ask me why I don't
know/can't remember).

clearing the cache if doesn't match is done by default, but if you
don't want that behavior you specify --skip-bad-cache

> 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.

yeah not sure why I didn't catch that, I did run the tests. I need to
revert part of this patch with the following patch


@@ -1122,7 +1164,7 @@
 	struct stat stat_file;
 	FILE * f = NULL;
 
-	if (cond_clear_cache && clear_cache_files(cacheloc) != 0)
+	if (clear_cache_files(cacheloc) != 0)
 		goto error;
 
 create_file:
@@ -1197,7 +1239,7 @@
 	get_flags_string(&cache_flags, cache_features_path);
 	if (cache_flags) {
 		if (strcmp(flags_string, cache_flags) != 0) {
-			if (write_cache) {
+			if (write_cache && cond_clear_cache) {
 				if (create_cache(cacheloc, cache_features_path,
 						 flags_string))
 					skip_read_cache = 1;

> 
>> ---
>> === 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:
> 
> 
> 
>