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.

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)

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

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

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)
     {

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);
 	}
 

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]);
 	    }
 	}

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