tar: --atime-preserve fixes for races etc.

This patch fixes a race condition in the --atime-preserve=replace
option, which might cause tar to improperly follow a symbolic link.

It also drops the use of the _FIOSATIME ioctl of Solaris 2.x
and later, which loses resolution on time stamps.  Modern Solaris
systems support full-resolution time stamps in the kernel, and
it's not worth the hassle of testing this call, useful only in
no-longer-supported Solaris variants.

Also, it undoes a change I recently introduced to the --compare
option, which caused it to not follow symbolic links unless the
--dereference option was also used.  Quite possibly this change is
a good idea, but the old behavior was documented and the change
should not have been installed casually.

* configure.ac: Don't check for stropts.h and sys/filio.h.
* gnulib.modules: Add futimens, utimensat.  Remove futimens.
* src/common.h (fd_utimensat): New decl.
* src/compare.c (diff_file, diff_multivol):
Don't use open_read_flags: those are for --create only.
* src/create.c (dump_file0): Adjust to set_file_atime changes.
Pass fstatat_flags to set_file_atime, so that symbolic links are
not followed inadvertantly.
* src/extract.c: Don't include utimens.h.
(set_stat): Use fd_utimensat ant UTIME_NOW rather than fdutimens.
* src/misc.c: Don't include utimens.h, stropts.h, sys/filio.h.
(fd_utimensat): New function.
(set_file_atime): Use it.  New arg atflag, controlling symlink
handling.  All callers changed.

diff --git a/configure.ac b/configure.ac
index 62df25fbec658d6e09fbbc48f1bef9621dcf9373..97a0869221b9ef05e3cee13efc05f4b0a3783fa9 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -40,8 +40,8 @@ AC_ISC_POSIX
 AC_C_INLINE
 
 AC_CHECK_HEADERS_ONCE(fcntl.h linux/fd.h memory.h net/errno.h \
-  sgtty.h string.h stropts.h \
-  sys/param.h sys/device.h sys/filio.h sys/gentape.h \
+  sgtty.h string.h \
+  sys/param.h sys/device.h sys/gentape.h \
   sys/inet.h sys/io/trioctl.h \
   sys/mtio.h sys/time.h sys/tprintf.h sys/tape.h \
   unistd.h locale.h)

diff --git a/gnulib.modules b/gnulib.modules
index a595d90288022d49427528d8ee3538772e0b9227..c6ded15f1170397ff605fdc03d126a9663f68613 100644 (file)
--- a/gnulib.modules
+++ b/gnulib.modules
@@ -18,6 +18,7 @@ fnmatch-gnu
 fseeko
 ftruncate
 full-write
+futimens
 getdate
 getline
 getopt-gnu
@@ -56,7 +57,7 @@ strtoul
 timespec
 unlinkdir
 unlocked-io
-utimens
+utimensat
 version-etc-fsf
 xalloc
 xalloc-die

diff --git a/src/common.h b/src/common.h
index e9088543894bea62c92d2a7093a424d92128c143..8bd86407dbfda3c8b24a726a7f76724fc0b517dd 100644 (file)
--- a/src/common.h
+++ b/src/common.h
@@ -610,6 +610,8 @@ bool maybe_backup_file (const char *file_name, bool this_is_the_archive);
 void undo_last_backup (void);
 
 int deref_stat (bool deref, char const *name, struct stat *buf);
+int fd_utimensat (int fd, int parentfd, char const *file,
+                 struct timespec const ts[2], int atflag);
 
 extern int chdir_current;
 int chdir_arg (char const *dir);
@@ -636,8 +638,8 @@ pid_t xfork (void);
 void xpipe (int fd[2]);
 
 void *page_aligned_alloc (void **ptr, size_t size);
-int set_file_atime (int fd, char const *file,
-                   struct timespec const timespec[2]);
+int set_file_atime (int fd, char const *file, struct timespec atime,
+                   int atflag);
 
 /* Module names.c.  */
 

diff --git a/src/compare.c b/src/compare.c
index b74793fc7e07f203af6918add5b8d80b83c8242e..6a873d7b131949bbc05fcb9634e78f90179fef5f 100644 (file)
--- a/src/compare.c
+++ b/src/compare.c
@@ -217,7 +217,14 @@ diff_file (void)
        }
       else
        {
-         diff_handle = open (file_name, open_read_flags);
+         int atime_flag =
+           (atime_preserve_option == system_atime_preserve
+            ? O_NOATIME
+            : 0);
+
+         diff_handle = open (file_name,
+                             (O_RDONLY | O_BINARY | O_CLOEXEC | O_NOCTTY
+                              | O_NONBLOCK | atime_flag));
 
          if (diff_handle < 0)
            {
@@ -236,10 +243,8 @@ diff_file (void)
 
              if (atime_preserve_option == replace_atime_preserve)
                {
-                 struct timespec ts[2];
-                 ts[0] = get_stat_atime (&stat_data);
-                 ts[1] = get_stat_mtime (&stat_data);
-                 if (set_file_atime (diff_handle, file_name, ts) != 0)
+                 struct timespec atime = get_stat_atime (&stat_data);
+                 if (set_file_atime (diff_handle, file_name, atime, 0) != 0)
                    utime_error (file_name);
                }
 
@@ -391,6 +396,10 @@ diff_multivol (void)
   struct stat stat_data;
   int fd, status;
   off_t offset;
+  int atime_flag =
+    (atime_preserve_option == system_atime_preserve
+     ? O_NOATIME
+     : 0);
 
   if (current_stat_info.had_trailing_slash)
     {
@@ -416,7 +425,10 @@ diff_multivol (void)
       return;
     }
 
-  fd = open (current_stat_info.file_name, open_read_flags);
+
+  fd = open (current_stat_info.file_name,
+            (O_RDONLY | O_BINARY | O_CLOEXEC | O_NOCTTY | O_NONBLOCK
+             | atime_flag));
 
   if (fd < 0)
     {

diff --git a/src/create.c b/src/create.c
index 9dc928d347839ad4c8bc63f59cd790c5d0fef341..6eedb2efc6e1dafb668311d8ffe01cbd45287e40 100644 (file)
--- a/src/create.c
+++ b/src/create.c
@@ -1610,7 +1610,6 @@ dump_file0 (struct tar_stat_info *st, char const *name, char const *p)
   char type;
   off_t original_size;
   struct timespec original_ctime;
-  struct timespec restore_times[2];
   off_t block_ordinal = -1;
   int fd = 0;
   bool is_dir;
@@ -1654,8 +1653,8 @@ dump_file0 (struct tar_stat_info *st, char const *name, char const *p)
     }
 
   st->archive_file_size = original_size = st->stat.st_size;
-  st->atime = restore_times[0] = get_stat_atime (&st->stat);
-  st->mtime = restore_times[1] = get_stat_mtime (&st->stat);
+  st->atime = get_stat_atime (&st->stat);
+  st->mtime = get_stat_mtime (&st->stat);
   st->ctime = original_ctime = get_stat_ctime (&st->stat);
 
 #ifdef S_ISHIDDEN
@@ -1794,7 +1793,7 @@ dump_file0 (struct tar_stat_info *st, char const *name, char const *p)
              set_exit_status (TAREXIT_DIFFERS);
            }
          else if (atime_preserve_option == replace_atime_preserve
-                  && set_file_atime (fd, p, restore_times) != 0)
+                  && set_file_atime (fd, p, st->atime, fstatat_flags) != 0)
            utime_error (p);
        }
 

diff --git a/src/extract.c b/src/extract.c
index 4bf27915169f7efe53a148b64fe5b8d332762576..2296066591624d6fb7bd8b8089632402cef3b225 100644 (file)
--- a/src/extract.c
+++ b/src/extract.c
@@ -21,7 +21,6 @@
 
 #include <system.h>
 #include <quotearg.h>
-#include <utimens.h>
 #include <errno.h>
 #include <priv-set.h>
 
@@ -304,24 +303,19 @@ set_stat (char const *file_name,
 
       if (! touch_option && permstatus != INTERDIR_PERMSTATUS)
        {
-         /* We set the accessed time to `now', which is really the time we
-            started extracting files, unless incremental_option is used, in
-            which case .st_atime is used.  */
-
-         /* FIXME: incremental_option should set ctime too, but how?  */
-
          struct timespec ts[2];
          if (incremental_option)
            ts[0] = st->atime;
          else
-           ts[0] = start_time;
+           ts[0].tv_nsec = UTIME_NOW;
          ts[1] = st->mtime;
 
-         if (fdutimens (file_name, fd, ts) != 0)
+         if (fd_utimensat (fd, AT_FDCWD, file_name, ts, 0) != 0)
            utime_error (file_name);
          else
            {
-             check_time (file_name, ts[0]);
+             if (incremental_option)
+               check_time (file_name, ts[0]);
              check_time (file_name, ts[1]);
            }
        }

diff --git a/src/misc.c b/src/misc.c
index 6f67887fab6d0f83829974d3eaa4b75353b399f7..1cf0c3b666054da93446f44ae94dff1ae53dc3c0 100644 (file)
--- a/src/misc.c
+++ b/src/misc.c
@@ -24,14 +24,6 @@
 #include <save-cwd.h>
 #include <xgetcwd.h>
 #include <unlinkdir.h>
-#include <utimens.h>
-
-#if HAVE_STROPTS_H
-# include <stropts.h>
-#endif
-#if HAVE_SYS_FILIO_H
-# include <sys/filio.h>
-#endif
 
 #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT
 # define DOUBLE_SLASH_IS_DISTINCT_ROOT 0
@@ -621,24 +613,30 @@ deref_stat (bool deref, char const *name, struct stat *buf)
   return deref ? stat (name, buf) : lstat (name, buf);
 }
 
-/* Set FD's (i.e., FILE's) access time to TIMESPEC[0].  If that's not
-   possible to do by itself, set its access and data modification
-   times to TIMESPEC[0] and TIMESPEC[1], respectively.  */
+/* Use futimens if possible, utimensat otherwise.  */
 int
-set_file_atime (int fd, char const *file, struct timespec const timespec[2])
+fd_utimensat (int fd, int parentfd, char const *file,
+             struct timespec const ts[2], int atflag)
 {
-#ifdef _FIOSATIME
   if (0 <= fd)
     {
-      struct timeval timeval;
-      timeval.tv_sec = timespec[0].tv_sec;
-      timeval.tv_usec = timespec[0].tv_nsec / 1000;
-      if (ioctl (fd, _FIOSATIME, &timeval) == 0)
-       return 0;
+      int result = futimens (fd, ts);
+      if (! (result < 0 && errno == ENOSYS))
+       return result;
     }
-#endif
 
-  return gl_futimens (fd, file, timespec);
+  return utimensat (parentfd, file, ts, atflag);
+}
+
+/* Set FD's (i.e., FILE's) access time to ATIME.
+   ATFLAG controls symbolic-link following, in the style of openat.  */
+int
+set_file_atime (int fd, char const *file, struct timespec atime, int atflag)
+{
+  struct timespec ts[2];
+  ts[0] = atime;
+  ts[1].tv_nsec = UTIME_OMIT;
+  return fd_utimensat (fd, AT_FDCWD, file, ts, atflag);
 }
 
 /* A description of a working directory.  */