Subject: bug#22770: closed (Re: bug#22768: Crash safety)

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 23 Feb 2016 07:29:02 +0000

Your bug report

#22768: Crash safety

which was filed against the gzip package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
22768: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22768
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#22768: Crash safety
From: Paul Eggert <[email-masked]>
To: Yanyan Jiang <[email-masked]>, [email-masked]
Date: Mon, 22 Feb 2016 23:28:07 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

Thanks for reporting the problem. It's annoying that gzip must invoke fsync, as 
that's way overkill compared to the write-ordering that is needed and fsync will 
slow gzip down, but I don't see any safe and reasonably portable alternative so 
I installed the attached patch on Savannah, here:

http://git.savannah.gnu.org/cgit/gzip.git/commit/?id=22aac8f8a616a72dbbe0e4119db8ddda0f076c04
From c8367d7660239b1aedce7850c766c0c0f8938d1c Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Mon, 22 Feb 2016 23:21:49 -0800
Subject: [PATCH] fsync output file before closing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Yanyan Jiang 蒋炎岩 in: http://bugs.gnu.org/22768
* NEWS: Document this.
* bootstrap.conf (gnulib_modules): Add fsync.
* gzip.c (treat_file): Call fsync just before closing the output.
* lib/.gitignore, m4/.gitignore: Add fsync-related gnulib files.
---
 NEWS           |  4 ++++
 bootstrap.conf |  1 +
 gzip.c         | 11 ++++++++++-
 lib/.gitignore |  1 +
 m4/.gitignore  |  1 +
 5 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index a1c668f..31472cc 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,10 @@ GNU gzip NEWS                                    -*- outline -*-
 
 ** Changes in behavior
 
+  When acting in-place, gzip now fsyncs the output before closing it.
+  This is slower, but on many file systems it is safer if the system
+  is about to crash.
+
   The GZIP environment variable is now obsolescent; gzip now warns if
   it is used, and rejects attempts to use dangerous options or operands.
   You can use an alias or script instead.
diff --git a/bootstrap.conf b/bootstrap.conf
index 13a485d..b15caa3 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -31,6 +31,7 @@ fcntl-safer
 fdl
 fdopendir
 fprintf-posix
+fsync
 getopt-gnu
 git-version-gen
 gitlog-to-changelog
diff --git a/gzip.c b/gzip.c
index a013540..b872383 100644
--- a/gzip.c
+++ b/gzip.c
@@ -926,8 +926,17 @@ local void treat_file(iname)
 
     if (!to_stdout)
       {
-
         copy_stat (&istat);
+
+        /* 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)
           write_error ();
 
diff --git a/lib/.gitignore b/lib/.gitignore
index 81d94ff..a368a26 100644
--- a/lib/.gitignore
+++ b/lib/.gitignore
@@ -225,3 +225,4 @@
 /xsize.h
 /yesno.c
 /yesno.h
+/fsync.c
diff --git a/m4/.gitignore b/m4/.gitignore
index 660b926..32f5566 100644
--- a/m4/.gitignore
+++ b/m4/.gitignore
@@ -150,3 +150,4 @@
 /xalloc.m4
 /xsize.m4
 /yesno.m4
+/fsync.m4
-- 
2.5.0

Subject: Crash safety
From: Yanyan Jiang <[email-masked]>
To: [email-masked]
Date: Mon, 22 Feb 2016 05:04:05 -0500
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi,

Gzip version: 1.6

I am developing a tool to validate crash safety of application software. I have just found that the file deletion has a potential safety venerability: if only a prefix of I/O operations are flushed to disk, after reboot, the file-system would only contain a 0-byte file (old file is deleted while the data is not reached to disk yet).

A paper FYI: http://0b4af6cdc2f0c5998459-c0245c5c937c5dedcca3f1764ecc9b2f.r43.cf2.rackcdn.com/17780-osdi14-paper-pillai.pdf <http://0b4af6cdc2f0c5998459-c0245c5c937c5dedcca3f1764ecc9b2f.r43.cf2.rackcdn.com/17780-osdi14-paper-pillai.pdf> (Table 1 on Page 440). Data append can be (virtually) reordered with any operation at default ext3 and ext4 settings. I recommend to use fsync() to persist the .gz file before deletion.

— strace log —

 36 open("a", O_RDONLY|O_NOCTTY|O_NONBLOCK|O_LARGEFILE|O_NOFOLLOW) = 3
 37 fstat64(3, {st_mode=S_IFREG|0664, st_size=19730, ...}) = 0
 38 rt_sigprocmask(SIG_BLOCK, [HUP INT PIPE TERM XCPU XFSZ], [], 8) = 0
 39 open("a.gz", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE, 0600) = 4
 40 rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
 41 read(3, "10017649652034232324895361757801"..., 65536) = 19730
 42 read(3, "", 45806)                      = 0
 43 write(4, "\37\213\10\10\24\312\312V\0\3a\0-\334m\226\234\274\22\3\340\377Y\r    \30l\354\375o,z\324"..., 9954) = 9954
 44 close(3)                                = 0
 45 utimensat(4, NULL, {{1456130580, 76955623}, {1456130580, 128955620}}, 0) = 0
 46 fchown32(4, 1000, 1000)                 = 0
 47 fchmod(4, 0664)                         = 0
 48 close(4)                                = 0
 49 rt_sigprocmask(SIG_BLOCK, [HUP INT PIPE TERM XCPU XFSZ], [], 8) = 0
 50 unlink("a")                             = 0

Thank you for your attention!

Regards,
Yanyan Jiang 蒋炎岩
Institute of Computer Software,
Dept. of Computer Science, Nanjing University