Subject: bug#20902: closed (Re: [BUG] hang at fifo despite of '-D skip')

From: [email-masked] (GNU bug Tracking System)
Date: Fri, 26 Jun 2015 23:52:02 +0000

Your bug report

#20902: [BUG] hang at fifo despite of '-D skip'

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

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

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

Subject: Re: [BUG] hang at fifo despite of '-D skip'
From: Paul Eggert <[email-masked]>
To: Uwe Koloska <[email-masked]>
Cc: [email-masked]
Date: Fri, 26 Jun 2015 16:50:52 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

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

Subject: [BUG] hang at fifo despite of '-D skip'
From: Uwe Koloska <[email-masked]>
To: [email-masked]
Date: Fri, 26 Jun 2015 11:40:29 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello,

'grep -D skip' hangs when examining a FIFO.

This is a regression. It has been working since 2.5.2 (commit
5c7194f9e739082dc14f75769f2c19a39bb6cf84).
Debian 6.0.10 has 2.6.3 and there it's working, too. But 2.12 in debian
7.8 has the problem.  Don't know exactly when it changed.

The help (and manual) states:
  -D, --devices=ACTION      how to handle devices, FIFOs and sockets;
                            ACTION is 'read' or 'skip'

>From this I expect, that grep will skip a FIFO and not block while
trying to handle it.

You can test for this error with this commands:

-8<-----------------------------------------------------
mkfifo myfifo
grep -D skip foo myfifo &
PID=$!
sleep 1
kill $PID 2>/dev/null && echo fail || echo ok
-8<-----------------------------------------------------

and here is the same idea as test for the test-suite:

#+FILE: tests/skip-fifo
-8<-----------------------------------------------------
#!/bin/sh
# grep must ignore a fifo when started with -D skip

. "${srcdir=.}/init.sh"; path_prepend_ ../src

mkfifo myfifo

grep -D skip foo myfifo &
PID=$!
# just to make sure
sleep 1
kill $PID && fail=1

# the other way round
grep foo myfifo &
PID=$!
# just to make sure
sleep 1
kill $PID || fail=1

Exit $fail
-8<-----------------------------------------------------

Best regards
Uwe Koloska