This is a note to let you know that I've just added the patch titled

    fifo: Do not restart open() if it already found a partner

to the 3.0-stable tree which can be found at:
    
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     fifo-do-not-restart-open-if-it-already-found-a-partner.patch
and it can be found in the queue-3.0 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <[email protected]> know about it.


>From 05d290d66be6ef77a0b962ebecf01911bd984a78 Mon Sep 17 00:00:00 2001
From: Anders Kaseorg <[email protected]>
Date: Sun, 15 Jul 2012 17:14:25 -0400
Subject: fifo: Do not restart open() if it already found a partner

From: Anders Kaseorg <[email protected]>

commit 05d290d66be6ef77a0b962ebecf01911bd984a78 upstream.

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 <[email protected]>
Reviewed-by: Jonathan Nieder <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
 fs/fifo.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

--- a/fs/fifo.c
+++ b/fs/fifo.c
@@ -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; 
 
@@ -23,6 +23,7 @@ static void wait_for_partner(struct inod
                if (signal_pending(current))
                        break;
        }
+       return cur == *cnt ? -ERESTARTSYS : 0;
 }
 
 static void wake_up_partner(struct inode* inode)
@@ -67,8 +68,7 @@ static int fifo_open(struct inode *inode
                                 * 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;
                        }
                }
@@ -90,8 +90,7 @@ static int fifo_open(struct inode *inode
                        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;


Patches currently in stable-queue which might be from [email protected] are

queue-3.0/fifo-do-not-restart-open-if-it-already-found-a-partner.patch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to