gzip: fdatasync output dir before unlinking
authorPaul Eggert <eggert@cs.ucla.edu>
Sat, 27 Feb 2016 22:12:46 +0000 (14:12 -0800)
committerPaul Eggert <eggert@cs.ucla.edu>
Sun, 28 Feb 2016 08:02:57 +0000 (00:02 -0800)
This follows up on the earlier patch to avoid data loss near
the system crashes.  See: http://bugs.gnu.org/22768
* bootstrap.conf (gnulib_modules): Add dirname-lgpl, fdatasync,
openat-safer, unistd-safer, unlinkat.
* gzip.c: Include stddef.h, dirname.h.
Include fcntl--.h instead of fcntl-safer.h.
(RW_USER): Remove; no longer needed.
(dfname, dfd): New static vars.
(dot): New static const.
(atdir_eq, atdir_set): New functions.
(treat_file): Also fdatasync the output directory, if !keep.
(treat_file, create_outfile, open_and_stat):
Use dir fd for unlinkat and openat, if possible.
(open_and_stat): Omit mode argument, since it was always the
same.  All callers changed.
* lib/.gitignore, m4/.gitignore: Add new gnulib files.
* tailor.h (PROTO, NO_STDIN_FSTAT, OPEN): Remove.  Remove MACOS
section, as this stuff would not work anyway now, and circa 2001
Apple stopped supporting Mac OS 9 and earlier.
* zip.c: Do not include unistd.h and fcntl.h, as this file does
not directly use any symbols defined by those headers.

bootstrap.conf
gzip.c
lib/.gitignore
m4/.gitignore
tailor.h
zip.c

index b15caa335ab4a032747da27df3d3ced6ae548d4c..308dc5edaebb2cb303a3173f966a2c45148803e1 100644 (file)
@@ -25,10 +25,12 @@ announce-gen
 calloc
 close
 closein
+dirname-lgpl
 fclose
 fcntl
 fcntl-safer
 fdl
+fdatasync
 fdopendir
 fprintf-posix
 fsync
@@ -47,6 +49,7 @@ lstat
 maintainer-makefile
 malloc-gnu
 manywarnings
+openat-safer
 perror
 printf-posix
 readme-release
@@ -55,6 +58,8 @@ savedir
 stat-time
 sys_stat
 time
+unistd-safer
+unlinkat
 update-copyright
 utimens
 xalloc
diff --git a/gzip.c b/gzip.c
index b872383def341073ac31948f5bee6d28faeee772..15bcf78f2da9258b4560fbf1d9d470fbda0ff4a1 100644 (file)
--- a/gzip.c
+++ b/gzip.c
@@ -59,6 +59,7 @@ static char const *const license_msg[] = {
 #include <sys/types.h>
 #include <signal.h>
 #include <stdbool.h>
+#include <stddef.h>
 #include <sys/stat.h>
 #include <errno.h>
 
@@ -70,7 +71,8 @@ static char const *const license_msg[] = {
 #include "revision.h"
 #include "timespec.h"
 
-#include "fcntl-safer.h"
+#include "dirname.h"
+#include "fcntl--.h"
 #include "getopt.h"
 #include "ignore-value.h"
 #include "stat-time.h"
@@ -79,7 +81,6 @@ static char const *const license_msg[] = {
 
                 /* configuration */
 
-#include <fcntl.h>
 #include <limits.h>
 #include <unistd.h>
 #include <stdlib.h>
@@ -97,8 +98,6 @@ static char const *const license_msg[] = {
 #  include <utimens.h>
 #endif
 
-#define RW_USER (S_IRUSR | S_IWUSR)  /* creation mode for open() */
-
 #ifndef MAX_PATH_LEN
 #  define MAX_PATH_LEN   1024 /* max pathname length */
 #endif
@@ -205,9 +204,11 @@ static off_t total_in;      /* input bytes for all files */
 static off_t total_out;            /* output bytes for all files */
 char ifname[MAX_PATH_LEN]; /* input file name */
 char ofname[MAX_PATH_LEN]; /* output file name */
+static char dfname[MAX_PATH_LEN]; /* name of dir containing output file */
 static struct stat istat;         /* status for input file */
 int  ifd;                  /* input file descriptor */
 int  ofd;                  /* output file descriptor */
+static int dfd = -1;       /* output directory file descriptor */
 unsigned insize;           /* valid bytes in inbuf */
 unsigned inptr;            /* index of next byte to be processed in inbuf */
 unsigned outcnt;           /* bytes in output buffer */
@@ -769,6 +770,48 @@ local void treat_stdin()
     }
 }
 
+static char const dot = '.';
+
+/* True if the cached directory for calls to openat etc. is DIR, with
+   length DIRLEN.  DIR need not be null-terminated.  DIRLEN must be
+   less than MAX_PATH_LEN.  */
+static bool
+atdir_eq (char const *dir, ptrdiff_t dirlen)
+{
+  if (dirlen == 0)
+    dir = &dot, dirlen = 1;
+  return memcmp (dfname, dir, dirlen) == 0 && !dfname[dirlen];
+}
+
+/* Set the directory used for calls to openat etc. to be the directory
+   DIR, with length DIRLEN.  DIR need not be null-terminated.
+   DIRLEN must be less than MAX_PATH_LEN.  Return a file descriptor for
+   the directory, or -1 if one could not be obtained.  */
+static int
+atdir_set (char const *dir, ptrdiff_t dirlen)
+{
+  /* Don't bother opening directories on older systems that
+     lack openat and unlinkat.  It's not worth the porting hassle.  */
+  #if HAVE_OPENAT && HAVE_UNLINKAT
+    enum { try_opening_directories = true };
+  #else
+    enum { try_opening_directories = false };
+  #endif
+
+  if (try_opening_directories && ! atdir_eq (dir, dirlen))
+    {
+      if (0 <= dfd)
+        close (dfd);
+      if (dirlen == 0)
+        dir = &dot, dirlen = 1;
+      memcpy (dfname, dir, dirlen);
+      dfname[dirlen] = '\0';
+      dfd = open (dfname, O_SEARCH | O_DIRECTORY);
+    }
+
+  return dfd;
+}
+
 /* ========================================================================
  * Compress or decompress the given file
  */
@@ -928,26 +971,30 @@ local void treat_file(iname)
       {
         copy_stat (&istat);
 
-        /* Transfer output data to the output file's storage device.
+        /* If KEEP, transfer output data to the output file's storage device.
            Otherwise, if the system crashed now the user might lose
            both input and output data.  See: Pillai TS et al.  All
            file systems are not created equal: on the complexity of
            crafting crash-consistent applications. OSDI'14. 2014:433-48.
            https://www.usenix.org/conference/osdi14/technical-sessions/presentation/pillai  */
-        if (!keep && fsync (ofd) != 0 && errno != EINVAL)
-          write_error ();
-
-        if (close (ofd) != 0)
+        if ((!keep
+             && ((0 <= dfd && fdatasync (dfd) != 0 && errno != EINVAL)
+                 || (fsync (ofd) != 0 && errno != EINVAL)))
+            || close (ofd) != 0)
           write_error ();
 
         if (!keep)
           {
             sigset_t oldset;
             int unlink_errno;
+            char *ifbase = last_component (ifname);
+            int ufd = atdir_eq (ifname, ifbase - ifname) ? dfd : -1;
+            int res;
 
             sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
             remove_ofname_fd = -1;
-            unlink_errno = xunlink (ifname) == 0 ? 0 : errno;
+            res = ufd < 0 ? xunlink (ifname) : unlinkat (ufd, ifbase, 0);
+            unlink_errno = res == 0 ? 0 : errno;
             sigprocmask (SIG_SETMASK, &oldset, NULL);
 
             if (unlink_errno)
@@ -998,6 +1045,19 @@ local int create_outfile()
   int name_shortened = 0;
   int flags = (O_WRONLY | O_CREAT | O_EXCL
                | (ascii && decompress ? 0 : O_BINARY));
+  char const *base = ofname;
+  int atfd = AT_FDCWD;
+
+  if (!keep)
+    {
+      char const *b = last_component (ofname);
+      int f = atdir_set (ofname, b - ofname);
+      if (0 <= f)
+        {
+          base = b;
+          atfd = f;
+        }
+    }
 
   for (;;)
     {
@@ -1005,7 +1065,7 @@ local int create_outfile()
       sigset_t oldset;
 
       sigprocmask (SIG_BLOCK, &caught_signals, &oldset);
-      remove_ofname_fd = ofd = OPEN (ofname, flags, RW_USER);
+      remove_ofname_fd = ofd = openat (atfd, base, flags, S_IRUSR | S_IWUSR);
       open_errno = errno;
       sigprocmask (SIG_SETMASK, &oldset, NULL);
 
@@ -1115,13 +1175,15 @@ local char *get_suffix(name)
 }
 
 
-/* Open file NAME with the given flags and mode and store its status
+/* Open file NAME with the given flags and store its status
    into *ST.  Return a file descriptor to the newly opened file, or -1
    (setting errno) on failure.  */
 static int
-open_and_stat (char *name, int flags, mode_t mode, struct stat *st)
+open_and_stat (char *name, int flags, struct stat *st)
 {
   int fd;
+  int atfd = AT_FDCWD;
+  char const *base = name;
 
   /* Refuse to follow symbolic links unless -c or -f.  */
   if (!to_stdout && !force)
@@ -1142,7 +1204,18 @@ open_and_stat (char *name, int flags, mode_t mode, struct stat *st)
         }
     }
 
-  fd = OPEN (name, flags, mode);
+  if (!keep)
+    {
+      char const *b = last_component (name);
+      int f = atdir_set (name, b - name);
+      if (0 <= f)
+        {
+          base = b;
+          atfd = f;
+        }
+    }
+
+  fd = openat (atfd, base, flags);
   if (0 <= fd && fstat (fd, st) != 0)
     {
       int e = errno;
@@ -1186,7 +1259,7 @@ open_input_file (iname, sbuf)
     strcpy(ifname, iname);
 
     /* If input file exists, return OK. */
-    fd = open_and_stat (ifname, open_flags, RW_USER, sbuf);
+    fd = open_and_stat (ifname, open_flags, sbuf);
     if (0 <= fd)
       return fd;
 
@@ -1227,7 +1300,7 @@ open_input_file (iname, sbuf)
         if (sizeof ifname <= ilen + strlen (s))
           goto name_too_long;
         strcat(ifname, s);
-        fd = open_and_stat (ifname, open_flags, RW_USER, sbuf);
+        fd = open_and_stat (ifname, open_flags, sbuf);
         if (0 <= fd)
           return fd;
         if (errno != ENOENT)
index a368a26efa2988ed55fdba3ab05dbda63155c981..b26889fb4d4b8ddf959d252bc4b238f0f6dc6d5e 100644 (file)
@@ -5,6 +5,7 @@
 /alloca.in.h
 /asnprintf.c
 /assure.h
+/at-func.c
 /basename-lgpl.c
 /c-ctype.c
 /c-ctype.h
@@ -59,6 +60,7 @@
 /fd-hook.c
 /fd-hook.h
 /fd-safer.c
+/fdatasync.c
 /fdopendir.c
 /fflush.c
 /filename.h
@@ -84,6 +86,8 @@
 /fseterr.c
 /fseterr.h
 /fstat.c
+/fstatat.c
+/fsync.c
 /ftell.c
 /ftello.c
 /getcwd-lgpl.c
 /openat-die.c
 /openat-priv.h
 /openat-proc.c
+/openat-safer.c
 /openat.c
 /openat.h
 /opendir-safer.c
 /ref-add.sin
 /ref-del.sed
 /ref-del.sin
+/rmdir.c
 /save-cwd.c
 /save-cwd.h
 /savedir.c
 /stat-time.c
 /stat-time.h
 /stat.c
+/statat.c
 /stdbool.h
 /stdbool.in.h
 /stddef.h
 /unistd.c
 /unistd.h
 /unistd.in.h
+/unlink.c
+/unlinkat.c
 /unused-parameter.h
 /utimens.c
 /utimens.h
 /xsize.h
 /yesno.c
 /yesno.h
-/fsync.c
index 32f5566bda9c98f7da28aa38e6435d8018fd8883..46415151d83815913c58943230a68775dc238193 100644 (file)
@@ -32,6 +32,7 @@
 /fcntl-safer.m4
 /fcntl.m4
 /fcntl_h.m4
+/fdatasync.m4
 /fdopendir.m4
 /fflush.m4
 /filenamecat.m4
@@ -48,6 +49,8 @@
 /fseeko.m4
 /fseterr.m4
 /fstat.m4
+/fstatat.m4
+/fsync.m4
 /ftell.m4
 /ftello.m4
 /getcwd.m4
 /quotearg.m4
 /readdir.m4
 /realloc.m4
+/rmdir.m4
 /save-cwd.m4
 /savedir.m4
 /signbit.m4
 /timespec.m4
 /unistd-safer.m4
 /unistd_h.m4
+/unlink.m4
+/unlinkat.m4
 /utimbuf.m4
 /utimens.m4
 /utimes.m4
 /xalloc.m4
 /xsize.m4
 /yesno.m4
-/fsync.m4
index 9d2399db7dd3b4bffc0c9f83bd3eb0065e182ad9..1feb807ca6dddd2cc38fa5f6ce82c86a83ce7899 100644 (file)
--- a/tailor.h
+++ b/tailor.h
@@ -56,7 +56,6 @@
 #  define NO_MULTIPLE_DOTS
 #  define MAX_EXT_CHARS 3
 #  define Z_SUFFIX "z"
-#  define PROTO
 #  define STDC_HEADERS
 #  define NO_SIZE_CHECK
 #  define UNLINK_READONLY_BUG
@@ -81,7 +80,6 @@
 #    define Z_SUFFIX "z"
 #    define casemap(c) tolow(c)
 #  endif
-#  define PROTO
 #  define STDC_HEADERS
 #  define UNLINK_READONLY_BUG
 #  include <io.h>
 #  define PATH_SEP2 '\\'
 #  define PATH_SEP3 ':'
 #  define MAX_PATH_LEN  260
-#  define PROTO
 #  define STDC_HEADERS
 #  define SET_BINARY_MODE(fd) setmode(fd, O_BINARY)
 #  define UNLINK_READONLY_BUG
 #    define HAVE_CHOWN
 #    define HAVE_LSTAT
 #  else /* SASC */
-#    define NO_STDIN_FSTAT
 #    define HAVE_SYS_DIR_H
 #    include <fcntl.h> /* for read() and write() */
 #    define direct dirent
 #  endif
 #endif
 
-#ifdef MACOS
-#  define PATH_SEP ':'
-#  define DYN_ALLOC
-#  define PROTO
-#  define NO_STDIN_FSTAT
-#  define chmod(file, mode) (0)
-#  define OPEN(name, flags, mode) open(name, flags)
-#  define OS_CODE  0x07
-#  ifdef MPW
-#    define isatty(fd) ((fd) <= 2)
-#  endif
-#endif
-
 #ifdef TOPS20
 #  define OS_CODE  0x0a
 #endif
 #ifndef SET_BINARY_MODE
 #  define SET_BINARY_MODE(fd)
 #endif
-
-#ifndef OPEN
-#  define OPEN(name, flags, mode) open_safer (name, flags, mode)
-#endif
diff --git a/zip.c b/zip.c
index 7f1117c5d23b35c328df395a1d09116d5920f782..178bfd0752d55629cfdb51f6be61999a079bb0ba 100644 (file)
--- a/zip.c
+++ b/zip.c
@@ -23,9 +23,6 @@
 #include "tailor.h"
 #include "gzip.h"
 
-#include <unistd.h>
-#include <fcntl.h>
-
 local ulg crc;       /* crc on uncompressed file data */
 off_t header_bytes;   /* number of bytes in gzip header */