On Tue, Jul 23, 2013 at 08:36:12AM -0700, 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 <[email protected]>

Acked-by: Seth Arnold <[email protected]>

Thanks

> 
> ---
>  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 {
> -- 
> Steve Beattie
> <[email protected]>
> http://NxNW.org/~steve/



> -- 
> AppArmor mailing list
> [email protected]
> Modify settings or unsubscribe at: 
> https://lists.ubuntu.com/mailman/listinfo/apparmor

Attachment: signature.asc
Description: Digital signature

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

Reply via email to