bug#24903: "tail -f - foo" does not terminate when stdin is closed and foo is ignored

[Julian Büning]

On 11/08/2016 06:47 PM, Pádraig Brady wrote:

p.s. These Symbolic Execution techniques are intriguing.
Have you any more details.


Thanks for your interest in our techniques. Symbolic Execution is a 
dynamic analysis method to automatically find bugs by executing a 
program with symbolic inputs, therefore reasoning about all possible 
execution paths. We based our work on KLEE [1], a symbolic execution 
engine, which has previously been used to find memory access violations 
in the GNU Coreutils suite [2]. We extended KLEE by adding the ability 
to automatically find unintended non-termination, such as the two bugs 
we reported on this mailing list. Unfortunately, I cannot share any 
details right now, as we are currently preparing a publication of our 
work. I will be happy to follow up once our paper has been published.


[1] http://klee.github.io
[2] http://llvm.org/pubs/2008-12-OSDI-KLEE.html

Regards,
Julian

bug#24903: "tail -f - foo" does not terminate when stdin is closed and foo is ignored

[Bernhard Voelker]

On 11/09/2016 12:44 AM, Bernhard Voelker wrote:
> and even more strange: when 'missing_file' then appears
> later, tail doesn't terminate when both files are removed:

oops, forget about this: we're following by descriptor, and
tail can not know if the file is still opened by other processes.
Sorry, ... time to go to bed.

So your conclusion seems correct: tail shouldn't re-try to
open("missing_file") in the -f case.

Have a nice day,
Berny

bug#24903: "tail -f - foo" does not terminate when stdin is closed and foo is ignored

[Bernhard Voelker]

On 11/09/2016 12:19 AM, Pádraig Brady wrote:
> +  tail -f - 'untailable file' will now terminate when there is no more data
> +  to read from stdin.  Previously it behaved as if --retry was specified.
> +  [This bug was present in "the beginning".]

LGTM, thanks.

> BTW I see a related issue with `tail -f missing_file existing_file`.
> Currently it will keep trying to open("missing_file") even though
> --retry is not specified.  That seems inconsistent to me.
> Shouldn't we ignore items that fail the initial open()?
> I'll do another fix for that unless there are objections.

Good catch - and even more strange: when 'missing_file' then appears
later, tail doesn't terminate when both files are removed:

  $ set -x; rm missing_file; \
  >   touch existing_file; \
  >   src/tail -f missing_file existing_file & \
  >   sleep 5; \
  >   echo abc > missing_file; \
  >   sleep 3; \
  >   rm missing_file existing_file; \
  >   sleep 3; \
  >   ps )
  + rm missing_file
  rm: cannot remove ‘missing_file’: No such file or directory
  + touch existing_file
  + sleep 5
  + src/tail -f missing_file existing_file
  src/tail: cannot open 'missing_file' for reading: No such file or directory
  ==> existing_file <==
  + echo abc
  src/tail: 'missing_file' has appeared;  following new file
  + sleep 3

  ==> missing_file <==
  abc
  + rm missing_file existing_file
  + sleep 3
  + ps
PID TTY  TIME CMD
   3036 pts/12   00:00:00 bash
   3039 pts/12   00:00:00 tail
   3053 pts/12   00:00:00 ps


Have a nice day,
Berny

bug#24903: "tail -f - foo" does not terminate when stdin is closed and foo is ignored

[Pádraig Brady]

On 08/11/16 21:28, Bernhard Voelker wrote:
> On 11/08/2016 06:47 PM, Pádraig Brady wrote:
>> Subject: [PATCH] tail: terminate when following pipes and untailable non 
>> pipes
>>
>> * src/tail.c (ignore_pipe_or_fifo): Mark the descriptor as -1
>> for pipes so that any_live_files() detects correctly that
>> the entry is no longer live.
>> * tests/tail-2/pipe-f.sh: Add a test case.
>> Fixes http://bugs.gnu.org/24903 which was detected
>> using Symbolic Execution techniques developed in
>> the course of the SYMBIOSYS research project at
>> COMSYS, RWTH Aachen University.
> 
> The patch looks good, yet I think it warrants a NEWS entry.

Oops. I've added:

+  tail -f - 'untailable file' will now terminate when there is no more data
+  to read from stdin.  Previously it behaved as if --retry was specified.
+  [This bug was present in "the beginning".]


BTW I see a related issue with `tail -f missing_file existing_file`.
Currently it will keep trying to open("missing_file") even though
--retry is not specified.  That seems inconsistent to me.
Shouldn't we ignore items that fail the initial open()?
I'll do another fix for that unless there are objections.

cheers,
Pádraig.

bug#24903: "tail -f - foo" does not terminate when stdin is closed and foo is ignored

[Bernhard Voelker]

On 11/08/2016 06:47 PM, Pádraig Brady wrote:
> Subject: [PATCH] tail: terminate when following pipes and untailable non pipes
> 
> * src/tail.c (ignore_pipe_or_fifo): Mark the descriptor as -1
> for pipes so that any_live_files() detects correctly that
> the entry is no longer live.
> * tests/tail-2/pipe-f.sh: Add a test case.
> Fixes http://bugs.gnu.org/24903 which was detected
> using Symbolic Execution techniques developed in
> the course of the SYMBIOSYS research project at
> COMSYS, RWTH Aachen University.

The patch looks good, yet I think it warrants a NEWS entry.

Thanks & have a nice day,
Berny

bug#24903: "tail -f - foo" does not terminate when stdin is closed and foo is ignored

[Pádraig Brady]

On 08/11/16 16:39, Julian Büning wrote:
> We observed another behavior possibly related to bug #24495:

Similar yes.

> $ mkdir foo
> $ echo "bar" | tail -f - foo &
> [1] 16386
> ==> standard input <==
> bar
> 
> ==> foo <==
> tail: error reading 'foo': Is a directory
> tail: foo: cannot follow end of this type of file; giving up on this name
> $ jobs
> [1]+  Running echo "bar" | tail -f - foo &

Yes we should exit here.

> $ readlink /proc/16386/fd/0
> pipe:[162156]
> $ lsof | grep 162156
> tail  16386  user0r FIFO   0,10   0t0
> 162156 pipe
> 
> Only the reading end of the pipe is still open, thus tail should not be
> able to read any more bytes from it.

Right we're not blocked on read()

The attached should fix it up.

thanks,
Pádraig.

p.s. These Symbolic Execution techniques are intriguing.
Have you any more details.
From a6207bdacee615ad6e19e24911aad7ae8364f6f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Tue, 8 Nov 2016 17:34:44 +
Subject: [PATCH] tail: terminate when following pipes and untailable non pipes

* src/tail.c (ignore_pipe_or_fifo): Mark the descriptor as -1
for pipes so that any_live_files() detects correctly that
the entry is no longer live.
* tests/tail-2/pipe-f.sh: Add a test case.
Fixes http://bugs.gnu.org/24903 which was detected
using Symbolic Execution techniques developed in
the course of the SYMBIOSYS research project at
COMSYS, RWTH Aachen University.
---
 src/tail.c |  5 -
 tests/tail-2/pipe-f.sh | 11 ++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/tail.c b/src/tail.c
index 718fc8a..96982ed 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -2212,7 +2212,10 @@ ignore_fifo_and_pipe (struct File_spec *f, size_t n_files)
  && (S_ISFIFO (f[i].mode)
  || (HAVE_FIFO_PIPES != 1 && isapipe (f[i].fd;
   if (is_a_fifo_or_pipe)
-f[i].ignore = true;
+{
+  f[i].fd = -1;
+  f[i].ignore = true;
+}
   else
 ++n_viable;
 }
diff --git a/tests/tail-2/pipe-f.sh b/tests/tail-2/pipe-f.sh
index 7abb7d6..82364da 100755
--- a/tests/tail-2/pipe-f.sh
+++ b/tests/tail-2/pipe-f.sh
@@ -19,9 +19,18 @@
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
 print_ver_ tail
 
+echo oo > exp || framework_failure_
 echo foo | timeout 10 tail -f -c3 > out || fail=1
-echo oo > exp || fail=1
+compare exp out || fail=1
+
+cat <<\EOF > exp
+==> standard input <==
+ar
 
+==> missing <==
+EOF
+mkdir missing || framework_failure_
+echo bar | returns_ 1 timeout 10 tail -f -c3 - missing > out || fail=1
 compare exp out || fail=1
 
 Exit $fail
-- 
2.5.5

bug#24903: "tail -f - foo" does not terminate when stdin is closed and foo is ignored

[Julian Büning]

We observed another behavior possibly related to bug #24495:

$ mkdir foo
$ echo "bar" | tail -f - foo &
[1] 16386
==> standard input <==
bar

==> foo <==
tail: error reading 'foo': Is a directory
tail: foo: cannot follow end of this type of file; giving up on this name
$ jobs
[1]+  Running echo "bar" | tail -f - foo &
$ readlink /proc/16386/fd/0
pipe:[162156]
$ lsof | grep 162156
tail  16386  user0r FIFO   0,10   0t0
162156 pipe

Only the reading end of the pipe is still open, thus tail should not be
able to read any more bytes from it.


expected behavior:

$ mkdir foo
$ echo "bar" | tail -f - foo &
[1] 16386
==> standard input <==
bar

==> foo <==
tail: error reading 'foo': Is a directory
tail: foo: cannot follow end of this type of file; giving up on this name
tail: no files remaining
[1]+  Doneecho "bar" | tail -f - foo &

This would match the behavior of tail when called without a directory as
parameter:

$ echo "bar" | tail -f - &
[1] 8411
bar
[1]+  Doneecho "bar" | tail -f -


We could reproduce this behavior with version 8.25 (package) and
8.25.91-23066 (compiled from source) on Fedora.

We need the directory (or some other untailable file) as second argument 
to reproduce this behavior as the -f option is ignored for any FIFO or 
pipe using ignore_fifo_and_pipe(), which prevents tail_forever() from 
being called in case only FIFOs or pipes are available. The 
aforementioned function sets .ignore to true for any FIFO or pipe.


In our test case, tail_forever() skips both the directory and stdin as
their .ignore entries are set to true. Before sleeping and starting the
next iteration of the loop (without making any attempt to read from
stdin), any_live_files() is called, which returns true for stdin:
 > if (0 <= f[i].fd)
 >return true;


This behavior was found using Symbolic Execution techniques developed in
the course of the SYMBIOSYS research project at COMSYS, RWTH Aachen
University.

Regards,
Julian Büning