xz: Use the self-pipe trick to avoid a race condition with signals.

It is possible that a signal to set user_abort arrives right
before a blocking system call is made. In this case the call
may block until another signal arrives, while the wanted
behavior is to make xz clean up and exit as soon as possible.

After this commit, the race condition is avoided with the
input side which already uses non-blocking I/O. The output
side still uses blocking I/O and thus has the race condition.

diff --git a/src/xz/file_io.c b/src/xz/file_io.c
index fd22dc4ffaf16deacc48c8b8dae4f9e780a9212b..21cdecb08f911ca0adca54527e847167323e90d8 100644 (file)
--- a/src/xz/file_io.c
+++ b/src/xz/file_io.c
@@ -51,6 +51,10 @@ static bool restore_stdin_flags = false;
 /// io_open_dest() and io_close_dest() to save and restore the flags.
 static int stdout_flags;
 static bool restore_stdout_flags = false;
+
+/// Self-pipe used together with the user_abort variable to avoid
+/// race conditions with signal handling.
+static int user_abort_pipe[2];
 #endif
 
 
@@ -70,6 +74,14 @@ io_init(void)
        // If fchown() fails setting the owner, we warn about it only if
        // we are root.
        warn_fchown = geteuid() == 0;
+
+       if (pipe(user_abort_pipe)
+                       || fcntl(user_abort_pipe[0], F_SETFL, O_NONBLOCK)
+                               == -1
+                       || fcntl(user_abort_pipe[1], F_SETFL, O_NONBLOCK)
+                               == -1)
+               message_fatal(_("Error creating a pipe: %s"),
+                               strerror(errno));
 #endif
 
 #ifdef __DJGPP__
@@ -82,6 +94,17 @@ io_init(void)
 }
 
 
+#ifndef TUKLIB_DOSLIKE
+extern void
+io_write_to_user_abort_pipe(void)
+{
+       uint8_t b = '\0';
+       (void)write(user_abort_pipe[1], &b, 1);
+       return;
+}
+#endif
+
+
 extern void
 io_no_sparse(void)
 {
@@ -91,11 +114,20 @@ io_no_sparse(void)
 
 
 #ifndef TUKLIB_DOSLIKE
-/// \brief      Waits for input or output to become available
+/// \brief      Waits for input or output to become available or for a signal
+///
+/// This uses the self-pipe trick to avoid a race condition that can occur
+/// if a signal is caught after user_abort has been checked but before e.g.
+/// read() has been called. In that situation read() could block unless
+/// non-blocking I/O is used. With non-blocking I/O something like select()
+/// or poll() is needed to avoid a busy-wait loop, and the same race condition
+/// pops up again. There are pselect() (POSIX-1.2001) and ppoll() (not in
+/// POSIX) but neither is portable enough in 2013. The self-pipe trick is
+/// old and very portable.
 static bool
 io_wait(file_pair *pair, bool is_reading)
 {
-       struct pollfd pfd[1];
+       struct pollfd pfd[2];
 
        if (is_reading) {
                pfd[0].fd = pair->src_fd;
@@ -105,18 +137,17 @@ io_wait(file_pair *pair, bool is_reading)
                pfd[0].events = POLLOUT;
        }
 
-       while (true) {
-               const int ret = poll(pfd, 1, -1);
+       pfd[1].fd = user_abort_pipe[0];
+       pfd[1].events = POLLIN;
 
-               if (ret == -1) {
-                       if (errno == EINTR) {
-                               if (user_abort)
-                                       return true;
+       while (true) {
+               const int ret = poll(pfd, 2, -1);
 
-                               continue;
-                       }
+               if (user_abort)
+                       return true;
 
-                       if (errno == EAGAIN)
+               if (ret == -1) {
+                       if (errno == EINTR || errno == EAGAIN)
                                continue;
 
                        message_error(_("%s: poll() failed: %s"),
@@ -125,7 +156,8 @@ io_wait(file_pair *pair, bool is_reading)
                                        strerror(errno));
                }
 
-               return false;
+               if (pfd[0].revents != 0)
+                       return false;
        }
 }
 #endif

diff --git a/src/xz/file_io.h b/src/xz/file_io.h
index ef639324894795daf866a4cc7076ea4396ff36eb..2de3379238d65f187fb574f671d0a51f4019dce5 100644 (file)
--- a/src/xz/file_io.h
+++ b/src/xz/file_io.h
@@ -68,6 +68,14 @@ typedef struct {
 extern void io_init(void);
 
 
+#ifndef TUKLIB_DOSLIKE
+/// \brief      Write a byte to user_abort_pipe[1]
+///
+/// This is called from a signal handler.
+extern void io_write_to_user_abort_pipe(void);
+#endif
+
+
 /// \brief      Disable creation of sparse files when decompressing
 extern void io_no_sparse(void);
 

diff --git a/src/xz/signals.c b/src/xz/signals.c
index de213644d00b2365bfbb1a79c08d29d3a3e32de5..2a1d4eb74c48746d8af48e39009c4230b8fe80c0 100644 (file)
--- a/src/xz/signals.c
+++ b/src/xz/signals.c
@@ -41,6 +41,11 @@ signal_handler(int sig)
 {
        exit_signal = sig;
        user_abort = true;
+
+#ifndef TUKLIB_DOSLIKE
+       io_write_to_user_abort_pipe();
+#endif
+
        return;
 }