[glibc/zack/y2038-preliminaries] Do not print backtraces on fatal glibc errors

[Zack Weinberg <zack at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=a289ea09ea843ced6e5277c2f2e63c357bc7f9a3

commit a289ea09ea843ced6e5277c2f2e63c357bc7f9a3
Author: Florian Weimer <fweimer@redhat.com>
Date:   Mon Aug 19 15:41:29 2019 +0200

    Do not print backtraces on fatal glibc errors
    
    If the process is in a bad state, we used to print backtraces in
    many cases.  This is problematic because doing so could involve
    a lot of work, like loading libgcc_s using the dynamic linker,
    and this could itself be targeted by exploit writers.  For example,
    if the crashing process was forked from a long-lived process, the
    addresses in the error message could be used to bypass ASLR.
    
    Commit ed421fca42fd9b4cab7c66e77894b8dd7ca57ed0 ("Avoid backtrace from
    __stack_chk_fail [BZ #12189]"), backtraces where no longer printed
    because backtrace_and_maps was always called with do_abort == 1.
    
    Rather than fixing this logic error, this change removes the backtrace
    functionality from the sources.  With the prevalence of external crash
    handlers, it does not appear to be particularly useful.  The crash
    handler may also destroy useful information for debugging.
    
    Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Diff:
---
 ChangeLog                            | 18 ++++++++++++++++++
 debug/fortify_fail.c                 | 26 +++-----------------------
 debug/stack_chk_fail.c               |  7 +------
 include/stdio.h                      |  4 ----
 sysdeps/posix/libc_fatal.c           | 35 ++++-------------------------------
 sysdeps/unix/sysv/linux/libc_fatal.c | 33 ---------------------------------
 6 files changed, 26 insertions(+), 97 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 73121e5..d0b235d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2019-08-19  Florian Weimer  <fweimer@redhat.com>
+
+	Do not print backtraces on fatal errors.
+	* debug/fortify_fail.c (__libc_argv): Remove declaration.
+	(__fortify_fail_abort): Remove definition.
+	(__fortify_fail): Call __libc_message directly.
+	* debug/stack_chk_fail.c (__libc_argv): Remove declaration.
+	(__stack_chk_fail): Call __fortify_fail instead of
+	__fortify_fail_abort.
+	* include/stdio.h (__fortify_fail_abort): Remove declaration.
+	* sysdeps/posix/libc_fatal.c (BEFORE_ABORT, before_abort): Remove
+	definitions.
+	(__libc_message): Do not handle do_backtrace.  Do not call
+	BEFORE_ABORT.
+	(__libc_fatal): Do not pass do_backtrace to __libc_message.
+	* sysdeps/unix/sysv/linux/libc_fatal.c (BEFORE_ABORT)
+	(before_abort): Remove definitions.
+
 2019-08-16  Florian Weimer  <fweimer@redhat.com>
 
 	nptl: Move pthread_attr_getdetachstate implementation into libc.
diff --git a/debug/fortify_fail.c b/debug/fortify_fail.c
index 16549d6..272a829 100644
--- a/debug/fortify_fail.c
+++ b/debug/fortify_fail.c
@@ -16,33 +16,13 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <stdio.h>
-#include <stdlib.h>
-#include <stdbool.h>
-
-
-extern char **__libc_argv attribute_hidden;
-
-void
-__attribute__ ((noreturn))
-__fortify_fail_abort (_Bool need_backtrace, const char *msg)
-{
-  /* The loop is added only to keep gcc happy.  Don't pass down
-     __libc_argv[0] if we aren't doing backtrace since __libc_argv[0]
-     may point to the corrupted stack.  */
-  while (1)
-    __libc_message (need_backtrace ? (do_abort | do_backtrace) : do_abort,
-		    "*** %s ***: %s terminated\n",
-		    msg,
-		    (need_backtrace && __libc_argv[0] != NULL
-		     ? __libc_argv[0] : "<unknown>"));
-}
 
 void
 __attribute__ ((noreturn))
 __fortify_fail (const char *msg)
 {
-  __fortify_fail_abort (true, msg);
+  /* The loop is added only to keep gcc happy.  */
+  while (1)
+    __libc_message (do_abort, "*** %s ***: terminated\n", msg);
 }
-
 libc_hidden_def (__fortify_fail)
-libc_hidden_def (__fortify_fail_abort)
diff --git a/debug/stack_chk_fail.c b/debug/stack_chk_fail.c
index 4485655..d4381df 100644
--- a/debug/stack_chk_fail.c
+++ b/debug/stack_chk_fail.c
@@ -16,17 +16,12 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <stdio.h>
-#include <stdlib.h>
-#include <stdbool.h>
-
-
-extern char **__libc_argv attribute_hidden;
 
 void
 __attribute__ ((noreturn))
 __stack_chk_fail (void)
 {
-  __fortify_fail_abort (false, "stack smashing detected");
+  __fortify_fail ("stack smashing detected");
 }
 
 strong_alias (__stack_chk_fail, __stack_chk_fail_local)
diff --git a/include/stdio.h b/include/stdio.h
index 5302e61..bea2066 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -102,7 +102,6 @@ enum __libc_message_action
 {
   do_message	= 0,		/* Print message.  */
   do_abort	= 1 << 0,	/* Abort.  */
-  do_backtrace	= 1 << 1	/* Backtrace.  */
 };
 
 /* Print out MESSAGE (which should end with a newline) on the error output
@@ -112,10 +111,7 @@ extern void __libc_fatal (const char *__message)
 extern void __libc_message (enum __libc_message_action action,
 			    const char *__fnt, ...) attribute_hidden;
 extern void __fortify_fail (const char *msg) __attribute__ ((__noreturn__));
-extern void __fortify_fail_abort (_Bool, const char *msg)
-  __attribute__ ((__noreturn__)) attribute_hidden;
 libc_hidden_proto (__fortify_fail)
-libc_hidden_proto (__fortify_fail_abort)
 
 /* Acquire ownership of STREAM.  */
 extern void __flockfile (FILE *__stream) attribute_hidden;
diff --git a/sysdeps/posix/libc_fatal.c b/sysdeps/posix/libc_fatal.c
index 3906af5..9ddbfa7 100644
--- a/sysdeps/posix/libc_fatal.c
+++ b/sysdeps/posix/libc_fatal.c
@@ -45,16 +45,6 @@ writev_for_fatal (int fd, const struct iovec *iov, size_t niov, size_t total)
 }
 #endif
 
-#ifndef BEFORE_ABORT
-# define BEFORE_ABORT		before_abort
-static void
-before_abort (int do_abort __attribute__ ((unused)),
-              bool written __attribute__ ((unused)),
-              int fd __attribute__ ((unused)))
-{
-}
-#endif
-
 struct str_list
 {
   const char *str;
@@ -75,17 +65,6 @@ __libc_message (enum __libc_message_action action, const char *fmt, ...)
   FATAL_PREPARE;
 #endif
 
-  /* Don't call __libc_secure_getenv if we aren't doing backtrace, which
-     may access the corrupted stack.  */
-  if ((action & do_backtrace))
-    {
-      /* Open a descriptor for /dev/tty unless the user explicitly
-	 requests errors on standard error.  */
-      const char *on_2 = __libc_secure_getenv ("LIBC_FATAL_STDERR_");
-      if (on_2 == NULL || *on_2 == '\0')
-	fd = __open_nocancel (_PATH_TTY, O_RDWR | O_NOCTTY | O_NDELAY);
-    }
-
   if (fd == -1)
     fd = STDERR_FILENO;
 
@@ -129,7 +108,6 @@ __libc_message (enum __libc_message_action action, const char *fmt, ...)
       ++nlist;
     }
 
-  bool written = false;
   if (nlist > 0)
     {
       struct iovec *iov = alloca (nlist * sizeof (struct iovec));
@@ -143,7 +121,7 @@ __libc_message (enum __libc_message_action action, const char *fmt, ...)
 	  list = list->next;
 	}
 
-      written = WRITEV_FOR_FATAL (fd, iov, nlist, total);
+      WRITEV_FOR_FATAL (fd, iov, nlist, total);
 
       if ((action & do_abort))
 	{
@@ -173,13 +151,8 @@ __libc_message (enum __libc_message_action action, const char *fmt, ...)
   va_end (ap);
 
   if ((action & do_abort))
-    {
-      if ((action & do_backtrace))
-	BEFORE_ABORT (do_abort, written, fd);
-
-      /* Kill the application.  */
-      abort ();
-    }
+    /* Kill the application.  */
+    abort ();
 }
 
 
@@ -188,6 +161,6 @@ __libc_fatal (const char *message)
 {
   /* The loop is added only to keep gcc happy.  */
   while (1)
-    __libc_message (do_abort | do_backtrace, "%s", message);
+    __libc_message (do_abort, "%s", message);
 }
 libc_hidden_def (__libc_fatal)
diff --git a/sysdeps/unix/sysv/linux/libc_fatal.c b/sysdeps/unix/sysv/linux/libc_fatal.c
index 56c6263..50a613e 100644
--- a/sysdeps/unix/sysv/linux/libc_fatal.c
+++ b/sysdeps/unix/sysv/linux/libc_fatal.c
@@ -17,11 +17,6 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <errno.h>
-#include <execinfo.h>
-#include <fcntl.h>
-#include <not-cancel.h>
-#include <string.h>
-#include <sys/mman.h>
 #include <sys/uio.h>
 
 static bool
@@ -37,32 +32,4 @@ writev_for_fatal (int fd, const struct iovec *iov, size_t niov, size_t total)
 }
 #define WRITEV_FOR_FATAL	writev_for_fatal
 
-static void
-backtrace_and_maps (int do_abort, bool written, int fd)
-{
-  if (do_abort > 1 && written)
-    {
-      void *addrs[64];
-#define naddrs (sizeof (addrs) / sizeof (addrs[0]))
-      int n = __backtrace (addrs, naddrs);
-      if (n > 2)
-        {
-#define strnsize(str) str, strlen (str)
-#define writestr(str) __write_nocancel (fd, str)
-          writestr (strnsize ("======= Backtrace: =========\n"));
-          __backtrace_symbols_fd (addrs + 1, n - 1, fd);
-
-          writestr (strnsize ("======= Memory map: ========\n"));
-          int fd2 = __open_nocancel ("/proc/self/maps", O_RDONLY);
-          char buf[1024];
-          ssize_t n2;
-          while ((n2 = __read_nocancel (fd2, buf, sizeof (buf))) > 0)
-            if (__write_nocancel (fd, buf, n2) != n2)
-              break;
-          __close_nocancel_nostatus (fd2);
-        }
-    }
-}
-#define BEFORE_ABORT		backtrace_and_maps
-
 #include <sysdeps/posix/libc_fatal.c>