Re: [PATCH v2] Improve ptrace-error detection on Linux targets

[Pedro Alves <palves at redhat dot com>]

Hi Sergio,

This is looking quite good to me.  Some comments below.

On 8/26/19 7:32 PM, Sergio Durigan Junior wrote:
> Changes from v1:
> 
> - Addressed Pedro's comments re. internal organization and gdbserver
>   support.
> 
> - Addressed Eli's comments (doc fixes).
> 
> - New ways of detecting what's wrong with ptrace.
> 
> 
> 
> In Fedora GDB, we carry the following patch:
> 
>   https://src.fedoraproject.org/rpms/gdb/blob/master/f/gdb-attach-fail-reasons-5of5.patch

This link will soon be dead, right?  (thinking about the commit log)
Maybe point at some reference other than master.

> Its purpose is to try to detect a specific scenario where SELinux's
> 'deny_ptrace' option is enabled, which prevents GDB from ptrace'ing in
> order to debug the inferior (PTRACE_ATTACH and PTRACE_ME will fail
> with EACCES in this case).
> 
> I like the idea of improving error detection and providing more
> information to the user (a simple "Permission denied" can be really
> frustrating), but I don't fully agree with the way the patch was
> implemented: it makes GDB link against libselinux only for the sake of
> consulting the 'deny_ptrace' setting, and then prints a warning if
> ptrace failed and this setting is on.
> 
> My first thought (and attempt) was to make GDB print a generic warning
> when a ptrace error happened; this message would just point the user
> to our documentation, where she could find more information about
> possible causes for the error (and try to diagnose/fix the problem).
> This proved to be too simple, and I was convinced that it is actually
> a good idea to go the extra kilometre and try to pinpoint the specific
> problem (or problems) preventing ptrace from working, as well as
> provide useful suggestions on how the user can fix things.
> 
> Here is the patch I came up with.  It implements a new function,
> 'linux_ptrace_restricted_fail_reason', which does a few things to
> check what's wrong with ptrace:
> 
>   - It dlopen's "libselinux.so" and checks if the "deny_ptrace" option
>     is enabled.

Update reference to "libselinux.so.1" here too.

> 
>   - It reads the contents of "/proc/sys/kernel/yama/ptrace_scope" and
>     checks if it's different than 0.
> 
> For each of these checks, if it succeeds, the user will see a message
> informing about the restriction in place, and how it can be disabled.
> For example, if "deny_ptrace" is enabled, the user will see:
> 
>   # gdb /bin/true
>   ...
>   Starting program: /usr/bin/true
>   warning: Could not trace the inferior process.
>   Error:
>   warning: ptrace: Permission denied

Curious, that:

 warning:
 Error:
 warning:

looks a bit odd, specifically the "Error:" line.  Do you know where is
that coming from?

>   The SELinux 'deny_ptrace' option is enabled and preventing GDB
>   from using 'ptrace'.  Please, disable it by executing (as root):

I'm really not a fan of these "Please, ".  Kind of sounds like
gdb is begging, to me.  I'd rather use an informational tone, like:

   The SELinux 'deny_ptrace' option is enabled and preventing GDB
   from using 'ptrace'.  You can disable it by executing (as root):

> 
>     setsebool deny_ptrace off
> 
>   During startup program exited with code 127.
> 
> In case "/proc/sys/kernel/yama/ptrace_scope" is > 0:
> 
>   # gdb /bin/true
>   ...
>   Starting program: /usr/bin/true
>   warning: Could not trace the inferior process.
>   Error:
>   warning: ptrace: Operation not permitted
>   The Linux kernel's Yama ptrace scope is in effect, which can prevent
>   GDB from using 'ptrace'.  Please, disable it by executing (as root):

Ditto.

> 
>     echo 0 > /proc/sys/kernel/yama/ptrace_scope
> 
>   During startup program exited with code 127.
> 
> If both restrictions are enabled, both messages will show up.
> 
> This works for gdbserver as well, and actually fixes a latent bug I
> found: when ptrace is restricted, gdbserver would hang due to an
> unchecked ptrace call:
> 
>   # gdbserver :9988 /bin/true
>   gdbserver: linux_ptrace_test_ret_to_nx: Cannot PTRACE_TRACEME: Operation not permitted
>   gdbserver: linux_ptrace_test_ret_to_nx: status 256 is not WIFSTOPPED!
>   gdbserver: linux_ptrace_test_ret_to_nx: failed to kill child pid 2668100 No such process
>   [ Here you would have to issue a C-c ]
> 
> Now, you will see:
> 
>   # gdbserver :9988 /bin/true
>   gdbserver: linux_ptrace_test_ret_to_nx: Cannot PTRACE_TRACEME: Operation not permitted
>   gdbserver: linux_ptrace_test_ret_to_nx: status 256 is not WIFSTOPPED!
>   gdbserver: linux_ptrace_test_ret_to_nx: failed to kill child pid 2668100 No such process
>   gdbserver: Could not trace the inferior process.
>   Error:
>   gdbserver: ptrace: Operation not permitted
>   The Linux kernel's Yama ptrace scope is in effect, which can prevent
>   GDB from using 'ptrace'.  Please, disable it by executing (as root):
> 
>     echo 0 > /proc/sys/kernel/yama/ptrace_scope
> 
>   linux_check_ptrace_features: waitpid: unexpected status 32512.
>   Exiting
> 
> (I decided to keep all the other messages, even though I find them a
> bit distracting).

Yeah, the other messages are implementor-speak, showing gdb function
names.  We should ideally clean this all up.

> 
> If GDB can't determine the cause for the failure, it will still print
> the generic error message which tells the user to check our
> documentation:
> 
>   There might be restrictions preventing ptrace from working.  Please see
>   the appendix "Linux kernel ptrace restrictions" in the GDB documentation
>   for more details.
> 
> This means that the patch expands our documentation and creates a new
> appendix section named "Linux kernel ptrace restrictions", with
> sub-sections for each possible restriction that might be in place.
> 
> The current list of possible restrictions is:
> 
>   - SELinux's 'deny_ptrace' option (detected).
> 
>   - YAMA's /proc/sys/kernel/yama/ptrace_scope setting (detected).
> 
>   - seccomp on Docker containers (I couldn't find how to detect).
> 
> It's important to mention that all of this is Linux-specific; as far
> as I know, SELinux, YAMA and seccomp are Linux-only features.
> 
> I tested this patch locally, on my Fedora 30 machine (actually, a
> Fedora Rawhide VM), but I'm not proposing a testcase for it because of
> the difficulty of writing one.
> 
> WDYT?
> 
> gdb/doc/ChangeLog:
> yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* gdb.texinfo (Linux kernel ptrace restrictions): New appendix
> 	section.
> 
> gdb/ChangeLog:
> yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>
> 	    Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdbsupport/gdb-dlfcn.c (gdb_dlopen): Add argument 'dont_throw'
> 	Don't throw if it's true.
> 	* gdbsupport/gdb-dlfcn.h (gdb_dlopen): Add optional argument
> 	'dont_throw'.  Update comment.
> 	* inf-ptrace.c (default_inf_ptrace_me_fail_reason): New
> 	function.
> 	(inf_ptrace_me_fail_reason): New variable.
> 	(inf_ptrace_me): Update call to 'trace_start_error_with_name'.
> 	* inf-ptrace.h (inf_ptrace_me_fail_reason): New variable.
> 	* linux-nat.c (linux_nat_target::attach): Update call to
> 	'linux_ptrace_attach_fail_reason'.
> 	(_initialize_linux_nat): Set 'inf_ptrace_me_fail_reason'.
> 	* linux-nat.h (linux_nat_target) <ptrace_me_fail_reason>: New
> 	method.
> 	* nat/fork-inferior.c (trace_start_error_with_name): Add
> 	optional 'append' argument.
> 	* nat/fork-inferior.h (trace_start_error_with_name): Update
> 	prototype.
> 	* nat/linux-ptrace.c: Include "gdbsupport/gdb-dlfcn.h" and
> 	"nat/fork-inferior.h".
> 	(selinux_ftype): New typedef.
> 	(linux_ptrace_restricted_fail_reason): New function.
> 	(linux_ptrace_attach_fail_reason): Add optional 'err'
> 	argument.  Call 'linux_ptrace_restricted_fail_reason'.
> 	(linux_ptrace_me_fail_reason): New function.
> 	(linux_child_function): Handle ptrace error.
> 	* nat/linux-ptrace.h (linux_ptrace_attach_fail_reason): Update
> 	prototype.
> 	(linux_ptrace_me_fail_reason): New function.
> 	* target-delegates.c: Regenerate.
> 	* target.h (struct target_ops) <ptrace_me_fail_reason>: New
> 	method.
> 	(target_ptrace_me_fail_reason): New define.
> 
> gdb/gdbserver/ChangeLog:
> yyyy-mm-dd  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* linux-low.c (linux_ptrace_fun): Call
> 	'linux_ptrace_me_fail_reason'.
> ---
>  gdb/doc/gdb.texinfo        | 138 +++++++++++++++++++++++++++++++++++++
>  gdb/gdbserver/linux-low.c  |   3 +-
>  gdb/gdbsupport/gdb-dlfcn.c |   4 +-
>  gdb/gdbsupport/gdb-dlfcn.h |   7 +-
>  gdb/inf-ptrace.c           |  19 ++++-
>  gdb/inf-ptrace.h           |  10 +++
>  gdb/linux-nat.c            |   7 +-
>  gdb/nat/fork-inferior.c    |   4 +-
>  gdb/nat/fork-inferior.h    |   7 +-
>  gdb/nat/linux-ptrace.c     |  86 +++++++++++++++++++++--
>  gdb/nat/linux-ptrace.h     |  15 +++-
>  11 files changed, 282 insertions(+), 18 deletions(-)
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index e1bc8143e6..43f749c6f1 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -182,6 +182,9 @@ software in general.  We will miss him.
>                                  @value{GDBN}
>  * Operating System Information:: Getting additional information from
>                                   the operating system
> +* Linux kernel @code{ptrace} restrictions:: Restrictions sometimes
> +                                            imposed by the Linux
> +                                            kernel on @code{ptrace}
>  * Trace File Format::		GDB trace file format
>  * Index Section Format::        .gdb_index section format
>  * Man Pages::			Manual pages
> @@ -44682,6 +44685,141 @@ should contain a comma-separated list of cores that this process
>  is running on.  Target may provide additional columns,
>  which @value{GDBN} currently ignores.
>  
> +@node Linux kernel ptrace restrictions
> +@appendix Linux kernel @code{ptrace} restrictions
> +@cindex linux kernel ptrace restrictions, attach
> +
> +The @code{ptrace} system call is used by @value{GDBN} on GNU/Linux to,
> +among other things, attach to a new or existing inferior in order to
> +start debugging it.  Due to security concerns, some distributions and
> +vendors disable or severily restrict the ability to perform these

typo: severily -> severely


> +operations, which can make @value{GDBN} malfunction.  In this section,
> +we will expand on how this malfunction can manifest itself, and how to
> +modify the system's settings in order to be able to use @value{GDBN}
> +properly.
> +
> +@menu
> +* The GDB error message::               The error message displayed when the
> +                                        system prevents @value{GDBN} from using
> +                                        @code{ptrace}
> +* SELinux's @code{deny_ptrace}::        SELinux and the @code{deny_ptrace} option
> +* Yama's @code{ptrace_scope}::          Yama and the @code{ptrace_scope} setting
> +* Docker and @code{seccomp}::           Docker and the @code{seccomp}
> +                                        infrastructure
> +@end menu
> +
> +@node The GDB error message
> +@appendixsection The @value{GDBN} error message
> +
> +When the system prevents @value{GDBN} from using the @code{ptrace}
> +system call, you will likely see a descriptive error message
> +explaining what is wrong and how to attempt to fix the problem.  For
> +example, when SELinux's @code{deny_ptrace} option is enabled, you can
> +see:
> +
> +@smallexample
> +$ gdb program
> +...
> +(@value{GDBP}) run
> +Starting program: program
> +warning: Could not trace the inferior process.
> +Error:
> +warning: ptrace: Permission denied
> +The SELinux 'deny_ptrace' option is enabled and preventing GDB
> +from using 'ptrace'.  Please, disable it by executing (as root):
> +
> +  setsebool deny_ptrace off
> +
> +During startup program exited with code 127.
> +(@value{GDBP})
> +@end smallexample
> +
> +Sometimes, it may not be possible to acquire the necessary data to
> +determine the root cause of the failure.  In this case, you will see a
> +generic error message pointing you to this section:
> +
> +@smallexample
> +$ gdb program
> +...
> +Starting program: program
> +warning: Could not trace the inferior process.
> +Error:
> +warning: ptrace: Permission denied
> +There might be restrictions preventing ptrace from working.  Please see
> +the appendix "Linux kernel ptrace restrictions" in the GDB documentation
> +for more details.
> +During startup program exited with code 127.
> +(@value{GDBP})
> +@end smallexample
> +
> +@node SELinux's deny_ptrace
> +@appendixsection SELinux's @code{deny_ptrace}
> +@cindex SELinux
> +@cindex deny_ptrace
> +
> +If you are using SELinux, you might want to check whether the
> +@code{deny_ptrace} option is enabled by doing:
> +
> +@smallexample
> +$ getsebool deny_ptrace
> +deny_ptrace --> on
> +@end smallexample
> +
> +If the option is enabled, you can disable it by doing, as root:
> +
> +@smallexample
> +# setsebool deny_ptrace off
> +@end smallexample
> +
> +The option will be disabled until the next reboot.  If you would like
> +to disable it permanently, you can do (as root):
> +
> +@smallexample
> +# setsebool -P deny_ptrace off
> +@end smallexample
> +
> +@node Yama's ptrace_scope
> +@appendixsection Yama's @code{ptrace_scope}
> +@cindex yama, ptrace_scope
> +
> +If your system has Yama enabled, you might want to check whether the
> +@code{ptrace_scope} setting is enabled by checking the value of
> +@file{/proc/sys/kernel/yama/ptrace_scope}:
> +
> +@smallexample
> +$ cat /proc/sys/kernel/yama/ptrace_scope
> +0
> +@end smallexample
> +
> +If you see anything other than @code{0}, @value{GDBN} can be affected
> +by it.  You can temporarily disable the feature by doing, as root:
> +
> +@smallexample
> +# sysctl kernel.yama.ptrace_scope=0
> +kernel.yama.ptrace_scope = 0
> +@end smallexample
> +
> +You can make this permanent by doing, as root:
> +
> +@smallexample
> +# sysctl -w kernel.yama.ptrace_scope=0
> +kernel.yama.ptrace_scope = 0
> +@end smallexample
> +
> +@node Docker and seccomp
> +@appendixsection Docker and @code{seccomp}
> +@cindex docker, seccomp
> +
> +If you are using Docker (@uref{https://www.docker.com/}) containers,
> +you will probably have to disable its @code{seccomp} protections in
> +order to be able to use @value{GDBN}.  To do that, you can use the
> +options @code{--cap-add=SYS_PTRACE --security-opt seccomp=unconfined}
> +when invoking Docker:
> +
> +@smallexample
> +$ docker run --cap-add=SYS_PTRACE --security-opt seccomp=unconfined
> +@end smallexample
> +
>  @node Trace File Format
>  @appendix Trace File Format
>  @cindex trace file format
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 3113017ae6..1e0a5cbf54 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -971,7 +971,8 @@ linux_ptrace_fun ()
>  {
>    if (ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0,
>  	      (PTRACE_TYPE_ARG4) 0) < 0)
> -    trace_start_error_with_name ("ptrace");
> +    trace_start_error_with_name ("ptrace",
> +				 linux_ptrace_me_fail_reason (errno).c_str ());
>  
>    if (setpgid (0, 0) < 0)
>      trace_start_error_with_name ("setpgid");
> diff --git a/gdb/gdbsupport/gdb-dlfcn.c b/gdb/gdbsupport/gdb-dlfcn.c
> index 921f10f3d8..9e5a992c17 100644
> --- a/gdb/gdbsupport/gdb-dlfcn.c
> +++ b/gdb/gdbsupport/gdb-dlfcn.c
> @@ -58,7 +58,7 @@ is_dl_available (void)
>  #else /* NO_SHARED_LIB */
>  
>  gdb_dlhandle_up
> -gdb_dlopen (const char *filename)
> +gdb_dlopen (const char *filename, bool dont_throw)
>  {
>    void *result;
>  #ifdef HAVE_DLFCN_H
> @@ -66,7 +66,7 @@ gdb_dlopen (const char *filename)
>  #elif __MINGW32__
>    result = (void *) LoadLibrary (filename);
>  #endif
> -  if (result != NULL)
> +  if (dont_throw || result != NULL)
>      return gdb_dlhandle_up (result);
>  
>  #ifdef HAVE_DLFCN_H
> diff --git a/gdb/gdbsupport/gdb-dlfcn.h b/gdb/gdbsupport/gdb-dlfcn.h
> index 6a39d38941..a8ddbc03da 100644
> --- a/gdb/gdbsupport/gdb-dlfcn.h
> +++ b/gdb/gdbsupport/gdb-dlfcn.h
> @@ -32,10 +32,11 @@ struct dlclose_deleter
>  typedef std::unique_ptr<void, dlclose_deleter> gdb_dlhandle_up;
>  
>  /* Load the dynamic library file named FILENAME, and return a handle
> -   for that dynamic library.  Return NULL if the loading fails for any
> -   reason.  */
> +   for that dynamic library.  If the loading fails, return NULL if
> +   DONT_THROW is true, or throw an exception otherwise (default
> +   behaviour).  */
>  
> -gdb_dlhandle_up gdb_dlopen (const char *filename);
> +gdb_dlhandle_up gdb_dlopen (const char *filename, bool dont_throw = false);
>  
>  /* Return the address of the symbol named SYMBOL inside the shared
>     library whose handle is HANDLE.  Return NULL when the symbol could
> diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
> index 4a8e732373..b6cfd803cf 100644
> --- a/gdb/inf-ptrace.c
> +++ b/gdb/inf-ptrace.c
> @@ -94,6 +94,22 @@ inf_ptrace_target::remove_fork_catchpoint (int pid)
>  #endif /* PT_GET_PROCESS_STATE */
>  
>  
> +/* Default method for "inf_ptrace_me_fail_reason", which returns an
> +   empty string.  */
> +
> +static std::string
> +default_inf_ptrace_me_fail_reason (int err)
> +{
> +  return {};
> +}
> +
> +/* Point to "inf_ptrace_me_fail_reason", 

Did you mean "Pointer"?  This reads strange because 
"inf_ptrace_me_fail_reason" is the name of the pointer itself,
so how to point _to_ it?

which implements a function
> +   that can be called by "inf_ptrace_me" in order to obtain the reason
> +   for failure.  */
> +
> +std::string (*inf_ptrace_me_fail_reason) (int err)
> +  = default_inf_ptrace_me_fail_reason;
> +
>  /* Prepare to be traced.  */
>  
>  static void
> @@ -101,7 +117,8 @@ inf_ptrace_me (void)
>  {
>    /* "Trace me, Dr. Memory!"  */
>    if (ptrace (PT_TRACE_ME, 0, (PTRACE_TYPE_ARG3) 0, 0) < 0)
> -    trace_start_error_with_name ("ptrace");
> +    trace_start_error_with_name ("ptrace",
> +				 inf_ptrace_me_fail_reason (errno).c_str ());
>  }
>  
>  /* Start a new inferior Unix child process.  EXEC_FILE is the file to
> diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h
> index 98b5d2e09e..7cdab9af89 100644
> --- a/gdb/inf-ptrace.h
> +++ b/gdb/inf-ptrace.h
> @@ -83,4 +83,14 @@ protected:
>  
>  extern pid_t get_ptrace_pid (ptid_t);
>  
> +/* Pointer to "inf_ptrace_me_fail_reason", which implements a function

Ah, here it's "pointer".  But still reads strange.  I think the comment
in the .c file should just say the usual "See inf-ptrace.h.".

> +   that can be called by "inf_ptrace_me" in order to obtain the reason
> +   for a ptrace failure.  ERR is the ERRNO value set by the failing
> +   ptrace call.
> +
> +   This pointer can be overriden by targets that want to personalize
> +   the error message printed when ptrace fails (see linux-nat.c, for
> +   example).  */
> +extern std::string (*inf_ptrace_me_fail_reason) (int err);
> +
>  #endif
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 945c19f666..b5a9eaf72e 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -1191,8 +1191,9 @@ linux_nat_target::attach (const char *args, int from_tty)
>      }
>    catch (const gdb_exception_error &ex)
>      {
> +      int saved_errno = errno;
>        pid_t pid = parse_pid_to_attach (args);
> -      std::string reason = linux_ptrace_attach_fail_reason (pid);
> +      std::string reason = linux_ptrace_attach_fail_reason (pid, saved_errno);
>  
>        if (!reason.empty ())
>  	throw_error (ex.error, "warning: %s\n%s", reason.c_str (),
> @@ -4696,6 +4697,10 @@ Enables printf debugging output."),
>    sigemptyset (&blocked_mask);
>  
>    lwp_lwpid_htab_create ();
> +
> +  /* Set the proper function to generate a message when ptrace
> +     fails.  */
> +  inf_ptrace_me_fail_reason = linux_ptrace_me_fail_reason;
>  }
>  
>  
> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
> index 68b51aa814..72ac623e20 100644
> --- a/gdb/nat/fork-inferior.c
> +++ b/gdb/nat/fork-inferior.c
> @@ -591,7 +591,7 @@ trace_start_error (const char *fmt, ...)
>  /* See nat/fork-inferior.h.  */
>  
>  void
> -trace_start_error_with_name (const char *string)
> +trace_start_error_with_name (const char *string, const char *append)
>  {
> -  trace_start_error ("%s: %s", string, safe_strerror (errno));
> +  trace_start_error ("%s: %s%s", string, safe_strerror (errno), append);
>  }
> diff --git a/gdb/nat/fork-inferior.h b/gdb/nat/fork-inferior.h
> index 1d0519fb26..7e6b889210 100644
> --- a/gdb/nat/fork-inferior.h
> +++ b/gdb/nat/fork-inferior.h
> @@ -98,9 +98,10 @@ extern void trace_start_error (const char *fmt, ...)
>    ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
>  
>  /* 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)
> +   combining STRING with the system error message for errno, and
> +   (optionally) with APPEND.  This function does not return.  */
> +extern void trace_start_error_with_name (const char *string,
> +					 const char *append = "")
>    ATTRIBUTE_NORETURN;
>  
>  #endif /* NAT_FORK_INFERIOR_H */
> diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
> index c1ebc0a032..599d9cfb55 100644
> --- a/gdb/nat/linux-ptrace.c
> +++ b/gdb/nat/linux-ptrace.c
> @@ -21,6 +21,8 @@
>  #include "linux-procfs.h"
>  #include "linux-waitpid.h"
>  #include "gdbsupport/buffer.h"
> +#include "gdbsupport/gdb-dlfcn.h"
> +#include "nat/fork-inferior.h"
>  #ifdef HAVE_SYS_PROCFS_H
>  #include <sys/procfs.h>
>  #endif
> @@ -30,11 +32,70 @@
>     of 0 means there are no supported features.  */
>  static int supported_ptrace_options = -1;
>  
> -/* Find all possible reasons we could fail to attach PID and return these
> -   as a string.  An empty string is returned if we didn't find any reason.  */
> +typedef int (*selinux_ftype) (const char *);
> +
> +/* Helper function which checks if ptrace is probably restricted
> +   (i.e., if ERR is either EACCES or EPERM), and returns a string with
> +   possible workarounds.  */
> +
> +static std::string
> +linux_ptrace_restricted_fail_reason (int err)
> +{
> +  if (err != EACCES && err != EPERM)
> +    {
> +      /* It just makes sense to perform the checks below if errno was
> +	 either EACCES or EPERM.  */
> +      return {};
> +    }
> +
> +  std::string ret;
> +  gdb_dlhandle_up handle = gdb_dlopen ("libselinux.so", true);
> +
> +  if (handle.get () != NULL)

No need for .get() here.  This:

  if (handle != NULL)

works the same.

(I'd write nullptr throughout instead of NULL in new code.)

> +    {
> +      selinux_ftype selinux_get_bool
> +	= (selinux_ftype) gdb_dlsym (handle, "security_get_boolean_active");
> +
> +      if (selinux_get_bool != NULL
> +	  && (*selinux_get_bool) ("deny_ptrace") == 1)
> +	string_appendf (ret,
> +			_("\n\
> +The SELinux 'deny_ptrace' option is enabled and preventing GDB\n\
> +from using 'ptrace'.  Please, disable it by executing (as root):\n\
> +\n\
> +  setsebool deny_ptrace off\n"));
> +    }
> +
> +  FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r");
> +

Use gdb_fopen_cloexec ?

> +  if (f != NULL)
> +    {
> +      char yama_scope = fgetc (f);
> +
> +      fclose (f);
> +
> +      if (yama_scope != '0')
> +	string_appendf (ret,
> +			_("\n\
> +The Linux kernel's Yama ptrace scope is in effect, which can prevent\n\
> +GDB from using 'ptrace'.  Please, disable it by executing (as root):\n\
> +\n\
> +  echo 0 > /proc/sys/kernel/yama/ptrace_scope\n"));
> +    }
> +
> +  if (ret.empty ())
> +    ret = _("\n\
> +There might be restrictions preventing ptrace from working.  Please see\n\
> +the appendix \"Linux kernel ptrace restrictions\" in the GDB documentation\n\
> +for more details.\n");
> +
> +  return ret;
> +}
> +
> +/* See declaration in linux-ptrace.h.  */
>  
>  std::string
> -linux_ptrace_attach_fail_reason (pid_t pid)
> +linux_ptrace_attach_fail_reason (pid_t pid, int err)
>  {
>    pid_t tracerpid = linux_proc_get_tracerpid_nowarn (pid);
>    std::string result;
> @@ -50,6 +111,11 @@ linux_ptrace_attach_fail_reason (pid_t pid)
>  		      "terminated"),
>  		    (int) pid);
>  
> +  std::string ptrace_restrict = linux_ptrace_restricted_fail_reason (err);
> +
> +  if (!ptrace_restrict.empty ())
> +    result += "\n" + ptrace_restrict;
> +
>    return result;
>  }
>  
> @@ -68,6 +134,14 @@ linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err)
>      return string_printf ("%s (%d)", safe_strerror (err), err);
>  }
>  
> +/* See linux-ptrace.h.  */
> +
> +std::string
> +linux_ptrace_me_fail_reason (int err)
> +{
> +  return linux_ptrace_restricted_fail_reason (err);
> +}
> +
>  #if defined __i386__ || defined __x86_64__
>  
>  /* Address of the 'ret' instruction in asm code block below.  */
> @@ -321,7 +395,11 @@ linux_grandchild_function (void *child_stack)
>  static int
>  linux_child_function (void *child_stack)
>  {
> -  ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) 0);
> +  if (ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0,
> +	      (PTRACE_TYPE_ARG4) 0) != 0)
> +    trace_start_error_with_name ("ptrace",
> +				 linux_ptrace_me_fail_reason (errno).c_str ());
> +
>    kill (getpid (), SIGSTOP);
>  
>    /* Fork a grandchild.  */
> diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
> index fd2f12a342..04ada53bf6 100644
> --- a/gdb/nat/linux-ptrace.h
> +++ b/gdb/nat/linux-ptrace.h
> @@ -176,13 +176,26 @@ struct buffer;
>  # define TRAP_HWBKPT 4
>  #endif
>  
> -extern std::string linux_ptrace_attach_fail_reason (pid_t pid);
> +/* Find all possible reasons we could fail to attach PID and return
> +   these as a string.  An empty string is returned if we didn't find
> +   any reason.  If ERR is EACCES or EPERM, we also add a warning about
> +   possible restrictions to use ptrace.  */
> +extern std::string linux_ptrace_attach_fail_reason (pid_t pid, int err = -1);

If ERR is an errno number, then it's a bit odd to use -1 for default,
since errno == 0 is the traditional "no error" number.  Pedantically, I believe
there's no garantee that a valid error number must be a positive integer.

But, why the default argument in the first place?  What calls this
without passing an error?

>  
>  /* Find all possible reasons we could have failed to attach to PTID
>     and return them as a string.  ERR is the error PTRACE_ATTACH failed
>     with (an errno).  */
>  extern std::string linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err);
>  
> +/* When the call to 'ptrace (PTRACE_TRACEME...' fails, and we have
> +   already forked, this function can be called in order to try to
> +   obtain the reason why ptrace failed.  ERR should be the ERRNO value
> +   returned by ptrace.
> +
> +   This function will return a 'std::string' containing the fail
> +   reason, or an empty string otherwise.  */
> +extern std::string linux_ptrace_me_fail_reason (int err);
> +
>  extern void linux_ptrace_init_warnings (void);
>  extern void linux_check_ptrace_features (void);
>  extern void linux_enable_event_reporting (pid_t pid, int attached);
> 

Thanks,
Pedro Alves