[apparmor] [parser patch] fix apparmor cache tempfile location to use passed arg

[John Johansen]

On 07/23/2013 08:36 AM, Steve Beattie wrote:
> Subject: [parser patch] fix apparmor cache tempfile location to use passed arg
> 
> This patch fixes problems in the handling of both the final cache
> name location and the temporary cache file when an alternate location
> is specified.
> 
> The first issue is that if the alternate cache directory location was
> specified, the alternate directory name would be used as the final location for
> the cache file, rather than the alternate directory + the basename of
> the profile.
> 
> The second issue is that it would generate the temporary file that it
> stores the cache file in [basedir]/cache even if an alternate cache
> location was specified on the command line. This causes a problem
> if [basedir]/cache is on a separate device than the alternate cache
> location, because the rename() of the tempfile into the final location
> would fail (which the parser would not check the return code of). It
> will also break if the filesystem the basedir is located on is mounted
> read-only, which would be a motivating reason to use an alternate
> cache location.
> 
> This patch fixes the above by incorporating the basename into the cache
> file name if the alternate cache location has been specified, bases the
> temporary cache file name on the destination cache name (such that they
> end up in the same directory), and finally detects if the rename fails
> and unlinks the temporary file if that happens (rather than leave it
> around).
> 
> Signed-off-by: Steve Beattie <sbeattie at ubuntu.com>
Acked-by: John Johansen <john.johansen at canonical.com>

> 
> ---
>  parser/parser_main.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> Index: b/parser/parser_main.c
> ===================================================================
> --- a/parser/parser_main.c
> +++ b/parser/parser_main.c
> @@ -1069,8 +1069,7 @@ int process_profile(int option, char *pr
>  	if ((profilename && option != OPTION_REMOVE) && !force_complain &&
>  	    !skip_cache) {
>  		if (cacheloc) {
> -			cachename = strdup(cacheloc);
> -			if (!cachename) {
> +			if (asprintf(&cachename, "%s/%s", cacheloc, basename)<0) {
>  				PERROR(_("Memory allocation error."));
>  				exit(1);
>  			}
> @@ -1089,7 +1088,7 @@ int process_profile(int option, char *pr
>  		}
>  		if (write_cache) {
>  			/* Otherwise, set up to save a cached copy */
> -			if (asprintf(&cachetemp, "%s/%s/%s-XXXXXX", basedir, "cache", basename)<0) {
> +			if (asprintf(&cachetemp, "%s-XXXXXX", cachename)<0) {
>  				perror("asprintf");
>  				exit(1);
>  			}
> @@ -1147,8 +1146,11 @@ out:
>  		}
>  
>  		if (useable_cache) {
> -			rename(cachetemp, cachename);
> -			if (show_cache)
> +			if (rename(cachetemp, cachename) < 0) {
> +				PERROR("Warning failed to write cache: %s\n", cachename);
> +				unlink(cachetemp);
> +			}
> +			else if (show_cache)
>  				PERROR("Wrote cache: %s\n", cachename);
>  		}
>  		else {
> 
> 
>