tail: don't call fstat on an uninitialized FD

This bug showed up via valgrind as a "Conditional jump or move
depends on uninitialized value(s)" error.
* src/tail.c (ignore_fifo_and_pipe): New function.
(main): Use it only when tailing forever.
The code to compute n_viable and mark some F[i] as ignored would call
isapipe on an uninitialized file descriptor.  But n_viable and those
.ignored marks are useful/used only when tailing forever.  This bug
was introduced via commit f0ff8c73 (7.6), "tail: make the new
piped-stdin test as portable as the old one".
* NEWS (Bug fixes): Mention it.

diff --git a/NEWS b/NEWS
index 02f2ce07574c262d2ebe419c530fb8c7d39441bd..1b56973de5b2eba4aef376d5799e1a91d1e0b4c9 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   if it uses helper processes for compression and its parent
   ignores CHLD signals. [bug introduced in coreutils-6.9]
 
+  tail without -f no longer access uninitialized memory
+  [bug introduced in coreutils-7.6]
+
   timeout is now immune to the signal handling of its parent.
   Specifically timeout now doesn't exit with an error message
   if its parent ignores CHLD signals. [bug introduced in coreutils-7.6]

diff --git a/src/tail.c b/src/tail.c
index 2bd9e3d654cecfc3a87e0398f057e6a6434a02a0..71f8a32d19dbddb9525a380293b05662703ff0ac 100644 (file)
--- a/src/tail.c
+++ b/src/tail.c
@@ -1889,6 +1889,35 @@ parse_options (int argc, char **argv,
     }
 }
 
+/* Mark as '.ignore'd each member of F that corresponds to a
+   pipe or fifo, and return the number of non-ignored members.  */
+static size_t
+ignore_fifo_and_pipe (struct File_spec *f, size_t n_files)
+{
+  /* When there is no FILE operand and stdin is a pipe or FIFO
+     POSIX requires that tail ignore the -f option.
+     Since we allow multiple FILE operands, we extend that to say: with -f,
+     ignore any "-" operand that corresponds to a pipe or FIFO.  */
+  size_t n_viable = 0;
+
+  size_t i;
+  for (i = 0; i < n_files; i++)
+    {
+      bool is_a_fifo_or_pipe =
+        (STREQ (f[i].name, "-")
+         && !f[i].ignore
+         && 0 <= f[i].fd
+         && (S_ISFIFO (f[i].mode)
+             || (HAVE_FIFO_PIPES != 1 && isapipe (f[i].fd))));
+      if (is_a_fifo_or_pipe)
+        f[i].ignore = true;
+      else
+        ++n_viable;
+    }
+
+  return n_viable;
+}
+
 int
 main (int argc, char **argv)
 {
@@ -1980,26 +2009,7 @@ main (int argc, char **argv)
   for (i = 0; i < n_files; i++)
     ok &= tail_file (&F[i], n_units);
 
-  /* When there is no FILE operand and stdin is a pipe or FIFO
-     POSIX requires that tail ignore the -f option.
-     Since we allow multiple FILE operands, we extend that to say:
-     ignore any "-" operand that corresponds to a pipe or FIFO.  */
-  size_t n_viable = 0;
-  for (i = 0; i < n_files; i++)
-    {
-      bool is_a_fifo_or_pipe =
-        (STREQ (F[i].name, "-")
-         && !F[i].ignore
-         && 0 <= F[i].fd
-         && (S_ISFIFO (F[i].mode)
-             || (HAVE_FIFO_PIPES != 1 && isapipe (F[i].fd))));
-      if (is_a_fifo_or_pipe)
-        F[i].ignore = true;
-      else
-        ++n_viable;
-    }
-
-  if (forever && n_viable)
+  if (forever && ignore_fifo_and_pipe (F, n_files))
     {
 #if HAVE_INOTIFY
       /* If the user specifies stdin via a command line argument of "-",