On Tue, Aug 11, 2015 at 09:53:53PM -0500, Tyler Hicks wrote:
> This patch fixes a regression in setting the cache file's timestamp
> handling that was introduced in r3079:
> 
>   Set cache file tstamp to the mtime of most recent policy file tstamp
> 
> The previously used utimes(2) syscall requires a two element timeval
> array. The first element in the array is the atime to be used and the
> second element is the mtime. The equivalent of a one element timeval
> array was being passed to it, resulting in garbage being used for the
> mtime value. The utimes(2) syscall either failed, due to the invalid
> input, or set mtime to an unexpected value. The return code wasn't being
> checked so the failure went unknown.
> 
> This patch switches to utimensat(2) for a couple reasons. The UTIME_OMIT
> special value allows us to preserve the inode's atime without calling
> stat(2) to fetch the atime. It also allows for nanosecond precision
> which better aligns with what stat(2) returns on the input profile and
> abstraction files. That means that we can have the exact same mtime on
> the input profile or abstraction file and the output cache file.
> 
> Signed-off-by: Tyler Hicks <[email protected]>

Nice catches all around.

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

Thanks

> ---
>  parser/policy_cache.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/parser/policy_cache.c b/parser/policy_cache.c
> index 4ba732c..9dd4b11 100644
> --- a/parser/policy_cache.c
> +++ b/parser/policy_cache.c
> @@ -18,6 +18,7 @@
>  
>  #include <ctype.h>
>  #include <dirent.h>
> +#include <fcntl.h>
>  #include <stdio.h>
>  #include <string.h>
>  #include <stdlib.h>
> @@ -25,8 +26,6 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <sys/stat.h>
> -#include <sys/time.h>
> -#include <utime.h>
>  
>  #include "lib.h"
>  #include "parser.h"
> @@ -166,12 +165,21 @@ void install_cache(const char *cachetmpname, const char 
> *cachename)
>       /* Only install the generate cache file if it parsed correctly
>          and did not have write/close errors */
>       if (cachetmpname) {
> -             struct timeval t;
> +             struct timespec times[2];
> +
>               /* set the mtime of the cache file to the most newest mtime
>                * of policy files used to generate it
>                */
> -             TIMESPEC_TO_TIMEVAL(&t, &mru_policy_tstamp);
> -             utimes(cachetmpname, &t);
> +             times[0].tv_sec = 0;
> +             times[0].tv_nsec = UTIME_OMIT;
> +             times[1] = mru_policy_tstamp;
> +             if (utimensat(AT_FDCWD, cachetmpname, times, 0) < 0) {
> +                     PERROR("%s: Failed to set the mtime of cache file '%s': 
> %m\n",
> +                            progname, cachename);
> +                     unlink(cachetmpname);
> +                     return;
> +             }
> +
>               if (rename(cachetmpname, cachename) < 0) {
>                       pwarn("Warning failed to write cache: %s\n", cachename);
>                       unlink(cachetmpname);

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