Subject: Re: [BUG] hang at fifo despite of '-D skip'

From: Paul Eggert <[email-masked]>
Date: Fri, 26 Jun 2015 16:50:52 -0700

Thanks for reporting this bug.  I installed the attached into the grep master on 
savannah, and it (or something like it) should appear in the next release.
>From bd6a91f0e891eaec4602b610426901d56b8aa3d7 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Fri, 26 Jun 2015 16:44:50 -0700
Subject: [PATCH] grep: don't hang on command-line fifo if -D skip

* NEWS: Document this.
* src/grep.c (skip_devices):
New function, with code taken from grepdirent.
(grepdirent): Use it.  Avoid an unnecessary initialization.
(grepfile): If skipping devices, open files with O_NONBLOCK.
Throw in O_NOCTTY while we're at it.
(grepdesc): Skip devices here, too.  Not only does this fix the
bug, it fixes an unlikely race condition if some other process
renames a device between fstatat and openat.
* tests/skip-device: Add a test for this bug.
---
 NEWS              |  3 +++
 src/grep.c        | 21 +++++++++++++++++----
 tests/skip-device |  5 +++++
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index ae8f38f..bbbe893 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,9 @@ GNU grep NEWS                                    -*- outline -*-
   When the JIT stack is exhausted, grep -P now grows the stack rather
   than reporting an internal PCRE error.
 
+  'grep -D skip PATTERN FILE' no longer hangs if FILE is a fifo.
+  [bug introduced in grep-2.12]
+
 
 * Noteworthy changes in release 2.21 (2014-11-23) [stable]
 
diff --git a/src/grep.c b/src/grep.c
index 6f43284..d1581e3 100644
--- a/src/grep.c
+++ b/src/grep.c
@@ -433,6 +433,13 @@ is_device_mode (mode_t m)
   return S_ISCHR (m) || S_ISBLK (m) || S_ISSOCK (m) || S_ISFIFO (m);
 }
 
+static bool
+skip_devices (bool command_line)
+{
+  return (devices == SKIP_DEVICES
+          || (devices == READ_COMMAND_LINE_DEVICES && !command_line));
+}
+
 /* Return if ST->st_size is defined.  Assume the file is not a
    symbolic link.  */
 static bool
@@ -1466,7 +1473,6 @@ grepdirent (FTS *fts, FTSENT *ent, bool command_line)
 {
   bool follow;
   int dirdesc;
-  struct stat *st = ent->fts_statp;
   command_line &= ent->fts_level == FTS_ROOTLEVEL;
 
   if (ent->fts_info == FTS_DP)
@@ -1515,9 +1521,9 @@ grepdirent (FTS *fts, FTSENT *ent, bool command_line)
 
     case FTS_DEFAULT:
     case FTS_NSOK:
-      if (devices == SKIP_DEVICES
-          || (devices == READ_COMMAND_LINE_DEVICES && !command_line))
+      if (skip_devices (command_line))
         {
+          struct stat *st = ent->fts_statp;
           struct stat st1;
           if (! st->st_mode)
             {
@@ -1572,7 +1578,10 @@ open_symlink_nofollow_error (int err)
 static bool
 grepfile (int dirdesc, char const *name, bool follow, bool command_line)
 {
-  int desc = openat_safer (dirdesc, name, O_RDONLY | (follow ? 0 : O_NOFOLLOW));
+  int oflag = (O_RDONLY | O_NOCTTY
+               | (follow ? 0 : O_NOFOLLOW)
+               | (skip_devices (command_line) ? O_NONBLOCK : 0));
+  int desc = openat_safer (dirdesc, name, oflag);
   if (desc < 0)
     {
       if (follow || ! open_symlink_nofollow_error (errno))
@@ -1601,6 +1610,10 @@ grepdesc (int desc, bool command_line)
       goto closeout;
     }
 
+  if (desc != STDIN_FILENO && skip_devices (command_line)
+      && is_device_mode (st.st_mode))
+    goto closeout;
+
   if (desc != STDIN_FILENO && command_line
       && skipped_file (filename, true, S_ISDIR (st.st_mode) != 0))
     goto closeout;
diff --git a/tests/skip-device b/tests/skip-device
index 32663fe..e73bad3 100755
--- a/tests/skip-device
+++ b/tests/skip-device
@@ -8,4 +8,9 @@
 echo foo | grep -D skip foo - || fail=1
 echo foo | grep --devices=skip foo || fail=1
 
+require_timeout_
+mkfifo myfifo || framework_failure_
+timeout 2s grep -D skip foo myfifo
+test $? -eq 1 || fail=1
+
 Exit $fail
-- 
2.1.0