[binutils-gdb] PR gdb/16188: Verify PTRACE_TRACEME succeeded

[Sergio Durigan Junior <sergiodj at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0db8980cc0ee05727c11f8b7c6674137a4d5de4e

commit 0db8980cc0ee05727c11f8b7c6674137a4d5de4e
Author: Sergio Durigan Junior <sergiodj@redhat.com>
Date:   Tue Feb 14 18:27:23 2017 -0500

    PR gdb/16188: Verify PTRACE_TRACEME succeeded
    
    This patch fixes PR gdb/16188, which is about the fact that
    fork_inferior doesn't verify the return value of the "traceme_fun"
    callback.  On most targets, this callback is actually a wrapper to a
    ptrace call that does a PTRACE_TRACEME on the forked GDB process that
    will eventually become the inferior.
    
    Thanks to Pedro, this second version of the patch is simpler and more
    more logical.  Basically, two helper functions are added:
    trace_start_error and trace_start_error_with_name.  The former can be
    used when there is a customized error message to be printed to the
    user.  The latter works like perror_with_name, so you just need to
    pass the function that error'd.
    
    Both helper functions mentioned above do basically the same thing:
    print the error message to stderr and call _exit, properly terminating
    the forked inferior.
    
    Most of the patch takes care of guarding the necessary system calls
    against errors on the "traceme_fun" callbacks.  It is not right to
    call error on these situations, so I've replaced these calls with the
    proper helper function call.
    
    Regression-tested on BuildBot.
    
    Thanks,
    
    gdb/ChangeLog:
    2017-02-20  Sergio Durigan Junior  <sergiodj@redhat.com>
    	    Pedro Alves  <palves@redhat.com>
    
    	PR gdb/16188
    	* darwin-nat.c (darwin_ptrace_me): Check if calls to system
    	calls succeeded.
    	* fork-child.c (trace_start_error): New function.
    	(trace_start_error_with_name): Likewise.
    	* gnu-nat.c (gnu_ptrace_me): Check if call to PTRACE succeeded.
    	* inf-ptrace.c (inf_ptrace_me): Likewise.
    	* inferior.h (trace_start_error): New prototype.
    	(trace_start_error_with_name): Likewise.

Diff:
---
 gdb/ChangeLog    | 13 +++++++++++++
 gdb/darwin-nat.c | 18 ++++++++++++------
 gdb/fork-child.c | 25 +++++++++++++++++++++++++
 gdb/gnu-nat.c    |  2 +-
 gdb/inf-ptrace.c |  3 ++-
 gdb/inferior.h   | 14 ++++++++++++++
 6 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4727433..cf68c7e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2017-02-20  Sergio Durigan Junior  <sergiodj@redhat.com>
+	    Pedro Alves  <palves@redhat.com>
+
+	PR gdb/16188
+	* darwin-nat.c (darwin_ptrace_me): Check if calls to system
+	calls succeeded.
+	* fork-child.c (trace_start_error): New function.
+	(trace_start_error_with_name): Likewise.
+	* gnu-nat.c (gnu_ptrace_me): Check if call to PTRACE succeeded.
+	* inf-ptrace.c (inf_ptrace_me): Likewise.
+	* inferior.h (trace_start_error): New prototype.
+	(trace_start_error_with_name): Likewise.
+
 2017-02-15  Sergio Durigan Junior  <sergiodj@redhat.com>
 
 	PR gdb/21164
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 8c5e8a0..edee1be 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1729,22 +1729,28 @@ darwin_ptrace_me (void)
   char c;
 
   /* Close write end point.  */
-  close (ptrace_fds[1]);
+  if (close (ptrace_fds[1]) < 0)
+    trace_start_error_with_name ("close");
 
   /* Wait until gdb is ready.  */
   res = read (ptrace_fds[0], &c, 1);
   if (res != 0)
-    error (_("unable to read from pipe, read returned: %d"), res);
-  close (ptrace_fds[0]);
+    trace_start_error (_("unable to read from pipe, read returned: %d"), res);
+
+  if (close (ptrace_fds[0]) < 0)
+    trace_start_error_with_name ("close");
 
   /* Get rid of privileges.  */
-  setegid (getgid ());
+  if (setegid (getgid ()) < 0)
+    trace_start_error_with_name ("setegid");
 
   /* Set TRACEME.  */
-  PTRACE (PT_TRACE_ME, 0, 0, 0);
+  if (PTRACE (PT_TRACE_ME, 0, 0, 0) < 0)
+    trace_start_error_with_name ("PTRACE");
 
   /* Redirect signals to exception port.  */
-  PTRACE (PT_SIGEXC, 0, 0, 0);
+  if (PTRACE (PT_SIGEXC, 0, 0, 0) < 0)
+    trace_start_error_with_name ("PTRACE");
 }
 
 /* Dummy function to be sure fork_inferior uses fork(2) and not vfork(2).  */
diff --git a/gdb/fork-child.c b/gdb/fork-child.c
index eaa8cb5..f6256fb 100644
--- a/gdb/fork-child.c
+++ b/gdb/fork-child.c
@@ -109,6 +109,31 @@ escape_bang_in_quoted_argument (const char *shell_file)
   return 0;
 }
 
+/* See inferior.h.  */
+
+void
+trace_start_error (const char *fmt, ...)
+{
+  va_list ap;
+
+  va_start (ap, fmt);
+  fprintf_unfiltered (gdb_stderr, "Could not trace the inferior "
+		                  "process.\nError: ");
+  vfprintf_unfiltered (gdb_stderr, fmt, ap);
+  va_end (args);
+
+  gdb_flush (gdb_stderr);
+  _exit (0177);
+}
+
+/* See inferior.h.  */
+
+void
+trace_start_error_with_name (const char *string)
+{
+  trace_start_error ("%s: %s", string, safe_strerror (errno));
+}
+
 /* Start an inferior Unix child process and sets inferior_ptid to its
    pid.  EXEC_FILE is the file to run.  ALLARGS is a string containing
    the arguments to the program.  ENV is the environment vector to
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 9935dcb..7efb3c1 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2126,7 +2126,7 @@ gnu_ptrace_me (void)
   struct inf *inf = cur_inf ();
   inf_debug (inf, "tracing self");
   if (ptrace (PTRACE_TRACEME) != 0)
-    error (_("ptrace (PTRACE_TRACEME) failed!"));
+    trace_start_error_with_name ("ptrace");
 }
 
 static void
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index f61bfe7..21578742 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -79,7 +79,8 @@ static void
 inf_ptrace_me (void)
 {
   /* "Trace me, Dr. Memory!"  */
-  ptrace (PT_TRACE_ME, 0, (PTRACE_TYPE_ARG3)0, 0);
+  if (ptrace (PT_TRACE_ME, 0, (PTRACE_TYPE_ARG3) 0, 0) < 0)
+    trace_start_error_with_name ("ptrace");
 }
 
 /* Start a new inferior Unix child process.  EXEC_FILE is the file to
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 258cc29..7c0ddf3 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -132,6 +132,20 @@ extern void child_terminal_init_with_pgrp (int pgrp);
 
 /* From fork-child.c */
 
+/* Report an error that happened when starting to trace the inferior
+   (i.e., when the "traceme_fun" callback is called on fork_inferior)
+   and bail out.  This function does not return.  */
+
+extern void trace_start_error (const char *fmt, ...)
+  ATTRIBUTE_NORETURN;
+
+/* Like "trace_start_error", but the error message is constructed by
+   combining STRING with the system error message for errno.  This
+   function does not return.  */
+
+extern void trace_start_error_with_name (const char *string)
+  ATTRIBUTE_NORETURN;
+
 extern int fork_inferior (char *, char *, char **,
 			  void (*)(void),
 			  void (*)(int), void (*)(void), char *,