Subject: Re: bug#25041: Bugs in TAC and TAIL for closed stdin

From: Pádraig Brady <[email-masked]>
Date: Sun, 27 Nov 2016 15:40:37 +0000

I'll push the attached later

thanks again,
Pádraiag
>From a31edf2aab384bfd33a6f0ab123d688939c4ddf6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Sun, 27 Nov 2016 13:00:35 +0000
Subject: [PATCH 1/2] tail: fix uninitialized memory read when failing to read
 file
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reproduced under UBSAN with `tail -f <&-` giving:
  tail.c:2220:18: runtime error: load of value 190,
  which is not a valid value for type ‘_Bool'

* src/tail.c (tail_file): Ensure f->ignore is initialized
in all cases where we can't tail the specified file.
* tests/tail-2/follow-stdin.sh: Add a test case which
checks stderr has no UBSAN warnings.
Fixes http://bugs.gnu.org/25041
---
 src/tail.c                   |  4 ++--
 tests/tail-2/follow-stdin.sh | 12 ++++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/tail.c b/src/tail.c
index 5c75be0..3d83550 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1940,8 +1940,6 @@ tail_file (struct File_spec *f, uintmax_t n_units)
               ok = false;
               f->errnum = -1;
               f->tailable = false;
-              f->ignore = ! (reopen_inaccessible_files
-                             && follow_mode == Follow_name);
               error (0, 0, _("%s: cannot follow end of this type of file%s"),
                      quotef (pretty_name (f)),
                      f->ignore ? _("; giving up on this name") : "");
@@ -1949,6 +1947,8 @@ tail_file (struct File_spec *f, uintmax_t n_units)
 
           if (!ok)
             {
+              f->ignore = ! (reopen_inaccessible_files
+                              && follow_mode == Follow_name);
               close_fd (fd, pretty_name (f));
               f->fd = -1;
             }
diff --git a/tests/tail-2/follow-stdin.sh b/tests/tail-2/follow-stdin.sh
index a2f1804..3d51f60 100755
--- a/tests/tail-2/follow-stdin.sh
+++ b/tests/tail-2/follow-stdin.sh
@@ -50,4 +50,16 @@ for mode in '' '---disable-inotify'; do
   cleanup_
 done
 
+
+# Before coreutils-8.26 this would induce an UMR under UBSAN
+returns_ 1 timeout 10 tail -f - <&- 2>err || fail=1
+cat <<\EOF >exp || framework_failure_
+tail: cannot fstat 'standard input': Bad file descriptor
+tail: error reading 'standard input': Bad file descriptor
+tail: no files remaining
+tail: -: Bad file descriptor
+EOF
+compare exp err || fail=1
+
+
 Exit $fail
-- 
2.5.5

>From 8fc0d1d68b37f67adad8ecc08b3875425130fcb5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Sun, 27 Nov 2016 15:09:53 +0000
Subject: [PATCH 2/2] tac: fix mem corruption when failing to read non seekable
 inputs

This was detected with ASAN, but can also be seen without ASAN with:
  $ tac - - <&-
  tac: standard input: read error: Bad file descriptor
  *** Error in `tac': malloc(): memory corruption: 0x...

* src/tac.c (copy_to_temp): Don't close our output stream on error;
including input errors.
* tests/misc/tac-2-nonseekable.sh: Add a test case.
Fixes http://bugs.gnu.org/25041
---
 src/tac.c                       | 10 +++-------
 tests/misc/tac-2-nonseekable.sh |  3 +++
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/tac.c b/src/tac.c
index 2e820fa..e9363b1 100644
--- a/src/tac.c
+++ b/src/tac.c
@@ -512,13 +512,13 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file)
       if (bytes_read == SAFE_READ_ERROR)
         {
           error (0, errno, _("%s: read error"), quotef (file));
-          goto Fail;
+          return -1;
         }
 
       if (fwrite (G_buffer, 1, bytes_read, fp) != bytes_read)
         {
           error (0, errno, _("%s: write error"), quotef (file_name));
-          goto Fail;
+          return -1;
         }
 
       /* Implicitly <= OFF_T_MAX due to preceding fwrite(),
@@ -530,16 +530,12 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file)
   if (fflush (fp) != 0)
     {
       error (0, errno, _("%s: write error"), quotef (file_name));
-      goto Fail;
+      return -1;
     }
 
   *g_tmp = fp;
   *g_tempfile = file_name;
   return bytes_copied;
-
- Fail:
-  fclose (fp);
-  return -1;
 }
 
 /* Copy INPUT_FD to a temporary, then tac that file.
diff --git a/tests/misc/tac-2-nonseekable.sh b/tests/misc/tac-2-nonseekable.sh
index 47e7849..08b35b3 100755
--- a/tests/misc/tac-2-nonseekable.sh
+++ b/tests/misc/tac-2-nonseekable.sh
@@ -36,4 +36,7 @@ for file in /proc/version /sys/kernel/profiling; do
   fi
 done
 
+# This failed due to heap corruption before coreutils 8.26
+returns_ 1 tac - - <&- 2>err || fail=1
+
 Exit $fail
-- 
2.5.5