>From 8604a5f86e8d1d83594b722a6e401f2e853cafa2 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 27 Feb 2016 14:12:46 -0800 Subject: [PATCH 1/4] gzip: fdatasync output dir before unlinking 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 | 5 +++ gzip.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++--------- lib/.gitignore | 10 +++++- m4/.gitignore | 7 +++- tailor.h | 21 ------------ zip.c | 3 -- 6 files changed, 109 insertions(+), 42 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index b15caa3..308dc5e 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -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 b872383..15bcf78 100644 --- a/gzip.c +++ b/gzip.c @@ -59,6 +59,7 @@ static char const *const license_msg[] = { #include #include #include +#include #include #include @@ -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 #include #include #include @@ -97,8 +98,6 @@ static char const *const license_msg[] = { # include #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) diff --git a/lib/.gitignore b/lib/.gitignore index a368a26..b26889f 100644 --- a/lib/.gitignore +++ b/lib/.gitignore @@ -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 @@ -132,6 +136,7 @@ /openat-die.c /openat-priv.h /openat-proc.c +/openat-safer.c /openat.c /openat.h /opendir-safer.c @@ -157,6 +162,7 @@ /ref-add.sin /ref-del.sed /ref-del.sin +/rmdir.c /save-cwd.c /save-cwd.h /savedir.c @@ -169,6 +175,7 @@ /stat-time.c /stat-time.h /stat.c +/statat.c /stdbool.h /stdbool.in.h /stddef.h @@ -205,6 +212,8 @@ /unistd.c /unistd.h /unistd.in.h +/unlink.c +/unlinkat.c /unused-parameter.h /utimens.c /utimens.h @@ -225,4 +234,3 @@ /xsize.h /yesno.c /yesno.h -/fsync.c diff --git a/m4/.gitignore b/m4/.gitignore index 32f5566..4641515 100644 --- a/m4/.gitignore +++ b/m4/.gitignore @@ -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 @@ -109,6 +112,7 @@ /quotearg.m4 /readdir.m4 /realloc.m4 +/rmdir.m4 /save-cwd.m4 /savedir.m4 /signbit.m4 @@ -136,6 +140,8 @@ /timespec.m4 /unistd-safer.m4 /unistd_h.m4 +/unlink.m4 +/unlinkat.m4 /utimbuf.m4 /utimens.m4 /utimes.m4 @@ -150,4 +156,3 @@ /xalloc.m4 /xsize.m4 /yesno.m4 -/fsync.m4 diff --git a/tailor.h b/tailor.h index 9d2399d..1feb807 100644 --- 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 @@ -114,7 +112,6 @@ # 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 @@ -179,7 +176,6 @@ # define HAVE_CHOWN # define HAVE_LSTAT # else /* SASC */ -# define NO_STDIN_FSTAT # define HAVE_SYS_DIR_H # include /* for read() and write() */ # define direct dirent @@ -202,19 +198,6 @@ # 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 @@ -276,7 +259,3 @@ #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 7f1117c..178bfd0 100644 --- a/zip.c +++ b/zip.c @@ -23,9 +23,6 @@ #include "tailor.h" #include "gzip.h" -#include -#include - local ulg crc; /* crc on uncompressed file data */ off_t header_bytes; /* number of bytes in gzip header */ -- 2.5.0