From: Paul Eggert Date: Thu, 16 Sep 2010 17:16:47 +0000 (-0700) Subject: tar: --atime-preserve fixes for races etc. X-Git-Url: https://git.dogcows.com/gitweb?p=chaz%2Ftar;a=commitdiff_plain;h=ecbcb7b6d74c2d69386c8d7e435486a4690c9993 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 62df25f..97a0869 100644 --- 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 a595d90..c6ded15 100644 --- 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 e908854..8bd8640 100644 --- 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 b74793f..6a873d7 100644 --- 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 9dc928d..6eedb2e 100644 --- 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 4bf2791..2296066 100644 --- a/src/extract.c +++ b/src/extract.c @@ -21,7 +21,6 @@ #include #include -#include #include #include @@ -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 6f67887..1cf0c3b 100644 --- a/src/misc.c +++ b/src/misc.c @@ -24,14 +24,6 @@ #include #include #include -#include - -#if HAVE_STROPTS_H -# include -#endif -#if HAVE_SYS_FILIO_H -# include -#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. */