Skip to content

Commit

Permalink
fifo: Do not restart open() if it already found a partner
Browse files Browse the repository at this point in the history
If a parent and child process open the two ends of a fifo, and the
child immediately exits, the parent may receive a SIGCHLD before its
open() returns.  In that case, we need to make sure that open() will
return successfully after the SIGCHLD handler returns, instead of
throwing EINTR or being restarted.  Otherwise, the restarted open()
would incorrectly wait for a second partner on the other end.

The following test demonstrates the EINTR that was wrongly thrown from
the parent’s open().  Change .sa_flags = 0 to .sa_flags = SA_RESTART
to see a deadlock instead, in which the restarted open() waits for a
second reader that will never come.  (On my systems, this happens
pretty reliably within about 5 to 500 iterations.  Others report that
it manages to loop ~forever sometimes; YMMV.)

  #include <sys/stat.h>
  #include <sys/types.h>
  #include <sys/wait.h>
  #include <fcntl.h>
  #include <signal.h>
  #include <stdio.h>
  #include <stdlib.h>
  #include <unistd.h>

  #define CHECK(x) do if ((x) == -1) {perror(#x); abort();} while(0)

  void handler(int signum) {}

  int main()
  {
      struct sigaction act = {.sa_handler = handler, .sa_flags = 0};
      CHECK(sigaction(SIGCHLD, &act, NULL));
      CHECK(mknod("fifo", S_IFIFO | S_IRWXU, 0));
      for (;;) {
          int fd;
          pid_t pid;
          putc('.', stderr);
          CHECK(pid = fork());
          if (pid == 0) {
              CHECK(fd = open("fifo", O_RDONLY));
              _exit(0);
          }
          CHECK(fd = open("fifo", O_WRONLY));
          CHECK(close(fd));
          CHECK(waitpid(pid, NULL, 0));
      }
  }

This is what I suspect was causing the Git test suite to fail in
t9010-svn-fe.sh:

	http://bugs.debian.org/678852

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Cc: stable@kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Anders Kaseorg authored and Linus Torvalds committed Jul 16, 2012
1 parent 1694a0a commit 05d290d
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions fs/fifo.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include <linux/sched.h>
#include <linux/pipe_fs_i.h>

static void wait_for_partner(struct inode* inode, unsigned int *cnt)
static int wait_for_partner(struct inode* inode, unsigned int *cnt)
{
int cur = *cnt;

Expand All @@ -23,6 +23,7 @@ static void wait_for_partner(struct inode* inode, unsigned int *cnt)
if (signal_pending(current))
break;
}
return cur == *cnt ? -ERESTARTSYS : 0;
}

static void wake_up_partner(struct inode* inode)
Expand Down Expand Up @@ -67,8 +68,7 @@ static int fifo_open(struct inode *inode, struct file *filp)
* seen a writer */
filp->f_version = pipe->w_counter;
} else {
wait_for_partner(inode, &pipe->w_counter);
if(signal_pending(current))
if (wait_for_partner(inode, &pipe->w_counter))
goto err_rd;
}
}
Expand All @@ -90,8 +90,7 @@ static int fifo_open(struct inode *inode, struct file *filp)
wake_up_partner(inode);

if (!pipe->readers) {
wait_for_partner(inode, &pipe->r_counter);
if (signal_pending(current))
if (wait_for_partner(inode, &pipe->r_counter))
goto err_wr;
}
break;
Expand Down

0 comments on commit 05d290d

Please sign in to comment.