[PATCH] Improve ptrace-error detection on Linux targets

classic Classic list List threaded Threaded
99 messages Options
12345
Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
On 8/29/19 8:48 PM, Sergio Durigan Junior wrote:

> You can see that we're calling "warning", which puts a newline at the
> end of the string.  I think it'd be best to just get rid of the
> "\nError: " suffix.  We'd then have something like:
>
>    Starting program: /usr/bin/true
>    warning: Could not trace the inferior process.
>    warning: ptrace: Permission denied
>    ....
>
> WDYT?

Sounds fine to me.

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

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

Sergio Durigan Junior
In reply to this post by Eli Zaretskii
On Friday, August 30 2019, Eli Zaretskii wrote:

>> Cc: Eli Zaretskii <[hidden email]>, Ruslan Kabatsayev <[hidden email]>
>> From: Pedro Alves <[hidden email]>
>> Date: Fri, 30 Aug 2019 13:47:06 +0100
>>
>> FYI, I'm seeing these warnings with your patch:
>>
>> $ make
>> makeinfo --split-size=5000000 -DHAVE_MAKEINFO_CLICK -I
>> /home/pedro/gdb/binutils-gdb/src/gdb/doc/../../readline/doc -I
>> /home/pedro/gdb/binutils-gdb/src/gdb/doc/../mi -I
>> /home/pedro/gdb/binutils-gdb/src/gdb/doc \
>>         -o gdb.info /home/pedro/gdb/binutils-gdb/src/gdb/doc/gdb.texinfo
>> /home/pedro/gdb/binutils-gdb/src/gdb/doc/gdb.texinfo:185: warning:
>> @menu entry node name `Linux kernel @code{ptrace} restrictions'
>> different from node name `Linux kernel ptrace restrictions'
>> /home/pedro/gdb/binutils-gdb/src/gdb/doc/gdb.texinfo:44712: warning: @menu entry node name `SELinux's @code{deny_ptrace}' different from node name `SELinux's deny_ptrace'
>> /home/pedro/gdb/binutils-gdb/src/gdb/doc/gdb.texinfo:44713: warning: @menu entry node name `Yama's @code{ptrace_scope}' different from node name `Yama's ptrace_scope'
>> /home/pedro/gdb/binutils-gdb/src/gdb/doc/gdb.texinfo:44714: warning: @menu entry node name `Docker and @code{seccomp}' different from node name `Docker and seccomp'
>
> We shouldn't use @-commands in node names.

But I fixed my patch, and v2 doesn't use @-commands in node names
anymore.

Hm, I think the problem is that I'm using @-commands inside @menu.  OK,
I'll fix that, and I'll also remove @-commands from @appendix.  Thanks
for pointing it out.

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Reply | Threaded
Open this post in threaded view
|

[PATCH] Remove "\nError: " suffix from nat/fork-inferior.c:trace_start_error warning message

Sergio Durigan Junior
In reply to this post by Sergio Durigan Junior
Rationale: https://sourceware.org/ml/gdb-patches/2019-08/msg00651.html

This very simple patch removes the "\nError: " suffix from the warning
message printed by nat/fork-inferior.c:trace_start_error.  This proved
to just pollute the screen, causing things like:

  Starting program: /usr/bin/true
  warning: Could not trace the inferior process.
  Error:
  warning: ptrace: Permission denied

This "Error: " string is not useful at all, and can confuse things,
therefore let's just remove it and simplify the resulting messages:

  Starting program: /usr/bin/true
  warning: Could not trace the inferior process.
  warning: ptrace: Permission denied

2019-08-29  Sergio Durigan Junior  <[hidden email]>

        * nat/fork-inferior.c (trace_start_error): Remove "\nError: "
        suffix from warning message.
---
 gdb/nat/fork-inferior.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 68b51aa814..355e9bef43 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -580,7 +580,7 @@ trace_start_error (const char *fmt, ...)
   va_list ap;
 
   va_start (ap, fmt);
-  warning ("Could not trace the inferior process.\nError: ");
+  warning ("Could not trace the inferior process.");
   vwarning (fmt, ap);
   va_end (ap);
 
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove "\nError: " suffix from nat/fork-inferior.c:trace_start_error warning message

Pedro Alves-7
On 8/30/19 8:51 PM, Sergio Durigan Junior wrote:

> Rationale: https://sourceware.org/ml/gdb-patches/2019-08/msg00651.html
>
> This very simple patch removes the "\nError: " suffix from the warning
> message printed by nat/fork-inferior.c:trace_start_error.  This proved
> to just pollute the screen, causing things like:
>
>   Starting program: /usr/bin/true
>   warning: Could not trace the inferior process.
>   Error:
>   warning: ptrace: Permission denied
>
> This "Error: " string is not useful at all, and can confuse things,
> therefore let's just remove it and simplify the resulting messages:
>
>   Starting program: /usr/bin/true
>   warning: Could not trace the inferior process.
>   warning: ptrace: Permission denied
>
> 2019-08-29  Sergio Durigan Junior  <[hidden email]>
>
> * nat/fork-inferior.c (trace_start_error): Remove "\nError: "
> suffix from warning message.

OK.

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove "\nError: " suffix from nat/fork-inferior.c:trace_start_error warning message

Sergio Durigan Junior
On Friday, August 30 2019, Pedro Alves wrote:

> On 8/30/19 8:51 PM, Sergio Durigan Junior wrote:
>> Rationale: https://sourceware.org/ml/gdb-patches/2019-08/msg00651.html
>>
>> This very simple patch removes the "\nError: " suffix from the warning
>> message printed by nat/fork-inferior.c:trace_start_error.  This proved
>> to just pollute the screen, causing things like:
>>
>>   Starting program: /usr/bin/true
>>   warning: Could not trace the inferior process.
>>   Error:
>>   warning: ptrace: Permission denied
>>
>> This "Error: " string is not useful at all, and can confuse things,
>> therefore let's just remove it and simplify the resulting messages:
>>
>>   Starting program: /usr/bin/true
>>   warning: Could not trace the inferior process.
>>   warning: ptrace: Permission denied
>>
>> 2019-08-29  Sergio Durigan Junior  <[hidden email]>
>>
>> * nat/fork-inferior.c (trace_start_error): Remove "\nError: "
>> suffix from warning message.
>
> OK.

Thanks, pushed: 47a536d940d2f2bccfec51539b857da06ebc429e

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Reply | Threaded
Open this post in threaded view
|

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

Sergio Durigan Junior
In reply to this post by Pedro Alves-7
On Friday, August 30 2019, Pedro Alves wrote:

> On 8/29/19 8:27 PM, Sergio Durigan Junior wrote:
>> On Thursday, August 29 2019, Pedro Alves wrote:
>
>>>> 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?
>>
>> So, the only place that calls linux_ptrace_attach_fail_reason without
>> passing the ERR argument is linux_ptrace_attach_fail_reason_string:
>>
>>   std::string
>>   linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err)
>>   {
>>     long lwpid = ptid.lwp ();
>>     std::string reason = linux_ptrace_attach_fail_reason (lwpid);
>>
>>     if (!reason.empty ())
>>       return string_printf ("%s (%d), %s", safe_strerror (err), err,
>>                             reason.c_str ());
>>     else
>>       return string_printf ("%s (%d)", safe_strerror (err), err);
>>   }
>>
>> In this case, I opted to keep it as is because the function will compose
>> a string contaning like:
>>
>>   A (B)[: C]
>>
>> Where:
>>
>>  A = safe_strerror
>>  B = errno
>>  C = fail reason (optional)
>>
>> This function (linux_ptrace_attach_fail_reason_string) is called in
>> three places:
>>
>> gdb/linux-nat.c:
>>
>>      std::string reason
>> = linux_ptrace_attach_fail_reason_string (ptid, err);
>>
>>      warning (_("Cannot attach to lwp %d: %s"),
>>       lwpid, reason.c_str ());
>>
>> gdb/gdbserver/linux-low.c:
>>
>>  std::string reason
>>    = linux_ptrace_attach_fail_reason_string (ptid, err);
>>
>>  warning (_("Cannot attach to lwp %d: %s"), lwpid, reason.c_str ());
>>
>> and
>>
>>       std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
>>       error ("Cannot attach to process %ld: %s", pid, reason.c_str ());
>>
>>
>> It seems to me like these error messages are expecting a short string to
>> just append to their existing strings, so I didn't think it made much
>> sense to extend the ptrace error checking here as well.  That's why I
>> didn't extend linux_ptrace_attach_fail_reason_string to pass ERR down to
>> linux_ptrace_attach_fail_reason.
>
> OK, I think that's just a bit too messy.  Let's take a fresh look at the
> result:
>
>  /* 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);
>
>  /* 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);
>
> Both the functions have the exact same prototype (same parameters, same return).
> And since they both return std::string, why is linux_ptrace_attach_fail_reason_string
> called "xxx_string"?  From the function names alone, I'd think
> linux_ptrace_attach_fail_reason_string returned a string, while
> linux_ptrace_attach_fail_reason returned something else, or printed
> directly.  But that's not the case.
>
> So linux_ptrace_attach_fail_reason_string is used when we're attaching
> to an lwp, other than the thread group leader (the main thread).
> IMO, it'd be better to rename that function accordingly, and update
> its comments accordingly too.
>
> Then, it becomes clearer that linux_ptrace_attach_fail_reason is to
> be used in the initial process attach, in which case we want to
> check the ptrace permissions.
>
> Also, we can factor out things such that we don't need the
> default parameter.  I tend to prefer avoiding mode-setting default
> parameters, preferring differently named functions, because default
> parameters make it harder to grep around the sources, distinguishing the
> cases where you passed an argument or not.  In this case,
> the linux_ptrace_attach_fail_reason / linux_ptrace_attach_fail_reason_string
> functions serve different purposes, so decoupling them via not using
> default parameters is better, IMO, it avoids missing some conversion
> in some spot.
>
> Which is exactly what happened.  Here's a patch implementing
> the idea.  Notice how gdbserver/linux-low.c:linux_attach
> has a spot where it is currently using linux_ptrace_attach_fail_reason_string
> after attaching to the main thread fails.  That spot should be including
> the new ptrace restrictions fail reasons, but it wasn't due to use of
> linux_ptrace_attach_fail_reason_string:
>
>  @@ -1202,7 +1202,7 @@ linux_attach (unsigned long pid)
>      {
>        remove_process (proc);
>  
>  -      std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
>  +      std::string reason = linux_ptrace_attach_fail_reason (pid, err);
>        error ("Cannot attach to process %ld: %s", pid, reason.c_str ());
>      }
>
> I haven't checked whether the resulting error message looks good as is,
> or whether we need to tweak that error call in addition.  Can you take it
> from here?
>
> Here's the patch on top of yours.

Sure, thanks for the patch.  I've tried it here and it seems fine.  I'll
submit it integrated into v3 soon.

>
> From a3e8744c1ed7fa8c702009fe3caf8578bb1785ea Mon Sep 17 00:00:00 2001
> From: Pedro Alves <[hidden email]>
> Date: Fri, 30 Aug 2019 13:15:12 +0100
> Subject: [PATCH] linux_ptrace_attach_fail_reason
>
> ---
>  gdb/gdbserver/linux-low.c |  4 ++--
>  gdb/gdbserver/thread-db.c |  2 +-
>  gdb/linux-nat.c           |  2 +-
>  gdb/nat/linux-ptrace.c    | 24 ++++++++++++++++++------
>  gdb/nat/linux-ptrace.h    | 10 ++++++----
>  5 files changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 1e0a5cbf54d..45cf43ad9ed 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -1170,7 +1170,7 @@ attach_proc_task_lwp_callback (ptid_t ptid)
>        else if (err != 0)
>   {
>    std::string reason
> -    = linux_ptrace_attach_fail_reason_string (ptid, err);
> +    = linux_ptrace_attach_fail_reason_lwp (ptid, err);
>  
>    warning (_("Cannot attach to lwp %d: %s"), lwpid, reason.c_str ());
>   }
> @@ -1202,7 +1202,7 @@ linux_attach (unsigned long pid)
>      {
>        remove_process (proc);
>  
> -      std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
> +      std::string reason = linux_ptrace_attach_fail_reason (pid, err);
>        error ("Cannot attach to process %ld: %s", pid, reason.c_str ());
>      }
>  
> diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
> index b2791b0513a..cfba05977e6 100644
> --- a/gdb/gdbserver/thread-db.c
> +++ b/gdb/gdbserver/thread-db.c
> @@ -225,7 +225,7 @@ attach_thread (const td_thrhandle_t *th_p, td_thrinfo_t *ti_p)
>    err = linux_attach_lwp (ptid);
>    if (err != 0)
>      {
> -      std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
> +      std::string reason = linux_ptrace_attach_fail_reason_lwp (ptid, err);
>  
>        warning ("Could not attach to thread %ld (LWP %d): %s",
>         (unsigned long) ti_p->ti_tid, ti_p->ti_lid, reason.c_str ());
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index b5a9eaf72e3..22cb55e6dcc 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -1136,7 +1136,7 @@ attach_proc_task_lwp_callback (ptid_t ptid)
>    else
>      {
>        std::string reason
> - = linux_ptrace_attach_fail_reason_string (ptid, err);
> + = linux_ptrace_attach_fail_reason_lwp (ptid, err);
>  
>        warning (_("Cannot attach to lwp %d: %s"),
>         lwpid, reason.c_str ());
> diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
> index 599d9cfb550..2201a24d812 100644
> --- a/gdb/nat/linux-ptrace.c
> +++ b/gdb/nat/linux-ptrace.c
> @@ -92,10 +92,13 @@ for more details.\n");
>    return ret;
>  }
>  
> -/* See declaration in linux-ptrace.h.  */
> +/* 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.  Helper for linux_ptrace_attach_fail_reason and
> +   linux_ptrace_attach_fail_reason_lwp.  */
>  
> -std::string
> -linux_ptrace_attach_fail_reason (pid_t pid, int err)
> +static std::string
> +linux_ptrace_attach_fail_reason_1 (pid_t pid)
>  {
>    pid_t tracerpid = linux_proc_get_tracerpid_nowarn (pid);
>    std::string result;
> @@ -111,8 +114,17 @@ linux_ptrace_attach_fail_reason (pid_t pid, int err)
>        "terminated"),
>      (int) pid);
>  
> -  std::string ptrace_restrict = linux_ptrace_restricted_fail_reason (err);
> +  return result;
> +}
>  
> +/* See declaration in linux-ptrace.h.  */
> +
> +std::string
> +linux_ptrace_attach_fail_reason (pid_t pid, int err)
> +{
> +  std::string result = linux_ptrace_attach_fail_reason_1 (pid);
> +
> +  std::string ptrace_restrict = linux_ptrace_restricted_fail_reason (err);
>    if (!ptrace_restrict.empty ())
>      result += "\n" + ptrace_restrict;
>  
> @@ -122,10 +134,10 @@ linux_ptrace_attach_fail_reason (pid_t pid, int err)
>  /* See linux-ptrace.h.  */
>  
>  std::string
> -linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err)
> +linux_ptrace_attach_fail_reason_lwp (ptid_t ptid, int err)
>  {
>    long lwpid = ptid.lwp ();
> -  std::string reason = linux_ptrace_attach_fail_reason (lwpid);
> +  std::string reason = linux_ptrace_attach_fail_reason_1 (lwpid);
>  
>    if (!reason.empty ())
>      return string_printf ("%s (%d), %s", safe_strerror (err), err,
> diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
> index 04ada53bf69..94c9ba48ba5 100644
> --- a/gdb/nat/linux-ptrace.h
> +++ b/gdb/nat/linux-ptrace.h
> @@ -180,12 +180,14 @@ struct buffer;
>     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);
> +extern std::string linux_ptrace_attach_fail_reason (pid_t pid, int err);
>  
> -/* Find all possible reasons we could have failed to attach to PTID
> +/* Find all possible reasons we could have failed to attach to LWPID
>     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);
> +   with (an errno).  Unlike linux_ptrace_attach_fail_reason, this
> +   function should be used when attaching to an LWP other than the
> +   leader; it does not warn about ptrace restrictions.  */
> +extern std::string linux_ptrace_attach_fail_reason_lwp (ptid_t lwpid, 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
> --
> 2.14.5

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Reply | Threaded
Open this post in threaded view
|

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

Sergio Durigan Junior
In reply to this post by Pedro Alves-7
On Friday, August 30 2019, Pedro Alves wrote:

> Which is exactly what happened.  Here's a patch implementing
> the idea.  Notice how gdbserver/linux-low.c:linux_attach
> has a spot where it is currently using linux_ptrace_attach_fail_reason_string
> after attaching to the main thread fails.  That spot should be including
> the new ptrace restrictions fail reasons, but it wasn't due to use of
> linux_ptrace_attach_fail_reason_string:
>
>  @@ -1202,7 +1202,7 @@ linux_attach (unsigned long pid)
>      {
>        remove_process (proc);
>  
>  -      std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
>  +      std::string reason = linux_ptrace_attach_fail_reason (pid, err);
>        error ("Cannot attach to process %ld: %s", pid, reason.c_str ());
>      }
>
> I haven't checked whether the resulting error message looks good as is,
> or whether we need to tweak that error call in addition.  Can you take it
> from here?

I forgot to say, but the way my patch works now prevents the code path
above to be executed.  My patch catches the ptrace failure very early in
gdbserver initialization the process, when linux_child_function is
called during the execution of linux_check_ptrace_features.  Since
linux_child_function will try to perform a PTRACE_TRACEME (and fail if
there are any restrictions in place), gdbserver will error out before it
even tries to spawn/attach.

Nevertheless, during my tests I bypassed linux_check_ptrace_features and
confirmed that the error message from linux_attach (above) is correctly
displayed:

  [sergio@fedora-rawhide build]$ sleep 120 &
  [5] 2732388
  [sergio@fedora-rawhide build]$ ./gdb/gdbserver/gdbserver :9911 --attach 2732388
  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 2732391 No such process
  Cannot attach to process 2732388:

  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

  The Linux kernel's Yama ptrace scope is in effect, which can prevent
  GDB from using 'ptrace'.  You can disable it by executing (as root):

    echo 0 > /proc/sys/kernel/yama/ptrace_scope

  Exiting

Thanks,

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Reply | Threaded
Open this post in threaded view
|

[PATCH v3] Improve ptrace-error detection on Linux targets

Sergio Durigan Junior
In reply to this post by Sergio Durigan Junior
Changes since v2:

- Fixed nits pointed by Pedro.  Incorporated his patch to improve the
  linux_ptrace_attach_fail_reason_* funtions.

- Fixed documentation issues.



In Fedora GDB, we carry the following patch:

  https://src.fedoraproject.org/rpms/gdb/blob/8ac06474ff1e2aa4920d14e0666b083eeaca8952/f/gdb-attach-fail-reasons-5of5.patch

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.1" and checks if the "deny_ptrace"
    option is enabled.

  - 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
  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'.  You can disable it by executing (as root):

    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'.  You can 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).

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  <[hidden email]>

        * gdb.texinfo (Linux kernel ptrace restrictions): New appendix
        section.

gdb/ChangeLog:
yyyy-mm-dd  Sergio Durigan Junior  <[hidden email]>
            Jan Kratochvil  <[hidden email]>
            Pedro Alves  <[hidden email]>

        * 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 (attach_proc_task_lwp_callback): Call
        'linux_ptrace_attach_fail_reason_lwp'.
        (linux_nat_target::attach): Update call to
        'linux_ptrace_attach_fail_reason'.
        (_initialize_linux_nat): Set 'inf_ptrace_me_fail_reason'.
        * 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",
        "gdbsupport/filestuff.h" and "nat/fork-inferior.h".
        (selinux_ftype): New typedef.
        (linux_ptrace_restricted_fail_reason): New function.
        (linux_ptrace_attach_fail_reason_1): New function.
        (linux_ptrace_attach_fail_reason): Change first argument type
        from 'ptid_t' to 'pid_t'.  Call
        'linux_ptrace_attach_fail_reason_1' and
        'linux_ptrace_restricted_fail_reason'.
        (linux_ptrace_attach_fail_reason_lwp): New function.
        (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_attach_fail_reason_lwp): New prototype.
        (linux_ptrace_me_fail_reason): New prototype.

gdb/gdbserver/ChangeLog:
yyyy-mm-dd  Sergio Durigan Junior  <[hidden email]>
            Pedro Alves  <[hidden email]>

        * linux-low.c (linux_ptrace_fun): Call
        'linux_ptrace_me_fail_reason'.
        (attach_proc_task_lwp_callback): Call
        'linux_ptrace_attach_fail_reason_lwp'.
        (linux_attach): Call 'linux_ptrace_attach_fail_reason'.
        * thread-db.c (attach_thread): Call
        'linux_ptrace_attach_fail_reason_lwp'.
---
 gdb/doc/gdb.texinfo        | 138 +++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/linux-low.c  |   7 +-
 gdb/gdbserver/thread-db.c  |   2 +-
 gdb/gdbsupport/gdb-dlfcn.c |   4 +-
 gdb/gdbsupport/gdb-dlfcn.h |   7 +-
 gdb/inf-ptrace.c           |  17 ++++-
 gdb/inf-ptrace.h           |  10 +++
 gdb/linux-nat.c            |   9 ++-
 gdb/nat/fork-inferior.c    |   4 +-
 gdb/nat/fork-inferior.h    |   7 +-
 gdb/nat/linux-ptrace.c     | 104 ++++++++++++++++++++++++++--
 gdb/nat/linux-ptrace.h     |  27 ++++++--
 12 files changed, 306 insertions(+), 30 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 53b7de91e4..16db62acac 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 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
@@ -44689,6 +44692,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 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 severely restrict the ability to perform these
+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 deny_ptrace::               SELinux and the @code{deny_ptrace} option
+* Yama's ptrace_scope::                 Yama and the @code{ptrace_scope} setting
+* Docker and 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'.  You can 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..45cf43ad9e 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");
@@ -1169,7 +1170,7 @@ attach_proc_task_lwp_callback (ptid_t ptid)
       else if (err != 0)
  {
   std::string reason
-    = linux_ptrace_attach_fail_reason_string (ptid, err);
+    = linux_ptrace_attach_fail_reason_lwp (ptid, err);
 
   warning (_("Cannot attach to lwp %d: %s"), lwpid, reason.c_str ());
  }
@@ -1201,7 +1202,7 @@ linux_attach (unsigned long pid)
     {
       remove_process (proc);
 
-      std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
+      std::string reason = linux_ptrace_attach_fail_reason (pid, err);
       error ("Cannot attach to process %ld: %s", pid, reason.c_str ());
     }
 
diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
index b2791b0513..cfba05977e 100644
--- a/gdb/gdbserver/thread-db.c
+++ b/gdb/gdbserver/thread-db.c
@@ -225,7 +225,7 @@ attach_thread (const td_thrhandle_t *th_p, td_thrinfo_t *ti_p)
   err = linux_attach_lwp (ptid);
   if (err != 0)
     {
-      std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
+      std::string reason = linux_ptrace_attach_fail_reason_lwp (ptid, err);
 
       warning ("Could not attach to thread %ld (LWP %d): %s",
        (unsigned long) ti_p->ti_tid, ti_p->ti_lid, reason.c_str ());
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..b792af00d1 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -94,6 +94,20 @@ 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 {};
+}
+
+/* See inf-ptrace.h.  */
+
+std::string (*inf_ptrace_me_fail_reason) (int err)
+  = default_inf_ptrace_me_fail_reason;
+
 /* Prepare to be traced.  */
 
 static void
@@ -101,7 +115,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
+   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..22cb55e6dc 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1136,7 +1136,7 @@ attach_proc_task_lwp_callback (ptid_t ptid)
   else
     {
       std::string reason
- = linux_ptrace_attach_fail_reason_string (ptid, err);
+ = linux_ptrace_attach_fail_reason_lwp (ptid, err);
 
       warning (_("Cannot attach to lwp %d: %s"),
        lwpid, reason.c_str ());
@@ -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 355e9bef43..2ead4a4858 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..0bbbe8d67a 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -21,6 +21,9 @@
 #include "linux-procfs.h"
 #include "linux-waitpid.h"
 #include "gdbsupport/buffer.h"
+#include "gdbsupport/gdb-dlfcn.h"
+#include "nat/fork-inferior.h"
+#include "gdbsupport/filestuff.h"
 #ifdef HAVE_SYS_PROCFS_H
 #include <sys/procfs.h>
 #endif
@@ -30,11 +33,72 @@
    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 *);
 
-std::string
-linux_ptrace_attach_fail_reason (pid_t pid)
+/* 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.1", true);
+
+  if (handle != nullptr)
+    {
+      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'.  You can disable it by executing (as root):\n\
+\n\
+  setsebool deny_ptrace off\n"));
+    }
+
+  gdb_file_up yama_ptrace_scope
+    = gdb_fopen_cloexec ("/proc/sys/kernel/yama/ptrace_scope", "r");
+
+  if (yama_ptrace_scope != nullptr)
+    {
+      char yama_scope = fgetc (yama_ptrace_scope.get ());
+
+      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'.  You can 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;
+}
+
+/* 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.  Helper for linux_ptrace_attach_fail_reason and
+   linux_ptrace_attach_fail_reason_lwp.  */
+
+static std::string
+linux_ptrace_attach_fail_reason_1 (pid_t pid)
 {
   pid_t tracerpid = linux_proc_get_tracerpid_nowarn (pid);
   std::string result;
@@ -56,10 +120,24 @@ linux_ptrace_attach_fail_reason (pid_t pid)
 /* See linux-ptrace.h.  */
 
 std::string
-linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err)
+linux_ptrace_attach_fail_reason (pid_t pid, int err)
+{
+  std::string result = linux_ptrace_attach_fail_reason_1 (pid);
+  std::string ptrace_restrict = linux_ptrace_restricted_fail_reason (err);
+
+  if (!ptrace_restrict.empty ())
+    result += "\n" + ptrace_restrict;
+
+  return result;
+}
+
+/* See linux-ptrace.h.  */
+
+std::string
+linux_ptrace_attach_fail_reason_lwp (ptid_t ptid, int err)
 {
   long lwpid = ptid.lwp ();
-  std::string reason = linux_ptrace_attach_fail_reason (lwpid);
+  std::string reason = linux_ptrace_attach_fail_reason_1 (lwpid);
 
   if (!reason.empty ())
     return string_printf ("%s (%d), %s", safe_strerror (err), err,
@@ -68,6 +146,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 +407,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..90afb60f34 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -176,12 +176,27 @@ struct buffer;
 # define TRAP_HWBKPT 4
 #endif
 
-extern std::string linux_ptrace_attach_fail_reason (pid_t pid);
-
-/* 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);
+/* 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);
+
+/* Find all possible reasons we could have failed to attach to PID's
+   LWPID and return them as a string.  ERR is the error PTRACE_ATTACH
+   failed with (an errno).  Unlike linux_ptrace_attach_fail_reason,
+   this function should be used when attaching to an LWP other than
+   the leader; it does not warn about ptrace restrictions.  */
+extern std::string linux_ptrace_attach_fail_reason_lwp (ptid_t pid, 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);
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
In reply to this post by Sergio Durigan Junior
On 9/4/19 8:31 PM, Sergio Durigan Junior wrote:

> I forgot to say, but the way my patch works now prevents the code path
> above to be executed.  My patch catches the ptrace failure very early in
> gdbserver initialization the process, when linux_child_function is
> called during the execution of linux_check_ptrace_features.  Since
> linux_child_function will try to perform a PTRACE_TRACEME (and fail if
> there are any restrictions in place), gdbserver will error out before it
> even tries to spawn/attach.
>
> Nevertheless, during my tests I bypassed linux_check_ptrace_features and
> confirmed that the error message from linux_attach (above) is correctly
> displayed:
>
>   [sergio@fedora-rawhide build]$ sleep 120 &
>   [5] 2732388
>   [sergio@fedora-rawhide build]$ ./gdb/gdbserver/gdbserver :9911 --attach 2732388
>   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 2732391 No such process
>   Cannot attach to process 2732388:
>
>   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
>
>   The Linux kernel's Yama ptrace scope is in effect, which can prevent
>   GDB from using 'ptrace'.  You can disable it by executing (as root):
>
>     echo 0 > /proc/sys/kernel/yama/ptrace_scope
>
>   Exiting

So if you _don't_ hack linux_check_ptrace_features, what does the user
see, with either

        gdbserver [OPTIONS] --attach COMM PID

or extended-remote + "(gdb) attach" ?

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

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

Sergio Durigan Junior
On Wednesday, September 04 2019, Pedro Alves wrote:

> On 9/4/19 8:31 PM, Sergio Durigan Junior wrote:
>> I forgot to say, but the way my patch works now prevents the code path
>> above to be executed.  My patch catches the ptrace failure very early in
>> gdbserver initialization the process, when linux_child_function is
>> called during the execution of linux_check_ptrace_features.  Since
>> linux_child_function will try to perform a PTRACE_TRACEME (and fail if
>> there are any restrictions in place), gdbserver will error out before it
>> even tries to spawn/attach.
>>
>> Nevertheless, during my tests I bypassed linux_check_ptrace_features and
>> confirmed that the error message from linux_attach (above) is correctly
>> displayed:
>>
>>   [sergio@fedora-rawhide build]$ sleep 120 &
>>   [5] 2732388
>>   [sergio@fedora-rawhide build]$ ./gdb/gdbserver/gdbserver :9911 --attach 2732388
>>   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 2732391 No such process
>>   Cannot attach to process 2732388:
>>
>>   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
>>
>>   The Linux kernel's Yama ptrace scope is in effect, which can prevent
>>   GDB from using 'ptrace'.  You can disable it by executing (as root):
>>
>>     echo 0 > /proc/sys/kernel/yama/ptrace_scope
>>
>>   Exiting
>
> So if you _don't_ hack linux_check_ptrace_features, what does the user
> see, with either
>
>         gdbserver [OPTIONS] --attach COMM PID

Basically the same thing:

  [sergio@fedora-rawhide build]$ ./gdb/gdbserver/gdbserver :9911 --attach 2733600
  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 2733602 No such process
  gdbserver: Could not trace the inferior process.
  gdbserver: ptrace: Operation not permitted
  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

  The Linux kernel's Yama ptrace scope is in effect, which can prevent
  GDB from using 'ptrace'.  You can disable it by executing (as root):

    echo 0 > /proc/sys/kernel/yama/ptrace_scope

  linux_check_ptrace_features: waitpid: unexpected status 32512.
  Exiting

The only difference is that we don't see the "Cannot attach to process
PID:" message.

> or extended-remote + "(gdb) attach" ?

I'm trying to come up with a way to test this.  The only way would be to
make gdbserver successfully start so that we could perform an
extended-remote connection from GDB.  However, if gdbserver starts
without problems, this means that the ptrace check succeeded and that
there are no ptrace restrictions in place.  Therefore, an "attach" would
succeed as well.  Which means that even if we're running GDB in a
ptrace-restricted system, things will go OK as long as gdbserver is not
restricted.  In this case, we wouldn't see any error messages IIUC.

Thanks,

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
On 9/4/19 9:21 PM, Sergio Durigan Junior wrote:
>> or extended-remote + "(gdb) attach" ?
> I'm trying to come up with a way to test this.  The only way would be to
> make gdbserver successfully start so that we could perform an
> extended-remote connection from GDB.  However, if gdbserver starts
> without problems, this means that the ptrace check succeeded and that
> there are no ptrace restrictions in place.  Therefore, an "attach" would
> succeed as well.  Which means that even if we're running GDB in a
> ptrace-restricted system, things will go OK as long as gdbserver is not
> restricted.  In this case, we wouldn't see any error messages IIUC.

So

 $ gdbserver --multi :9999

exits with error immediately?

You could start gdbserver with the restrictions off (like a long
lived daemon), and then while gdbserver is running enable
restrictions, I suppose.

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

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

Sergio Durigan Junior
On Wednesday, September 04 2019, Pedro Alves wrote:

> On 9/4/19 9:21 PM, Sergio Durigan Junior wrote:
>>> or extended-remote + "(gdb) attach" ?
>> I'm trying to come up with a way to test this.  The only way would be to
>> make gdbserver successfully start so that we could perform an
>> extended-remote connection from GDB.  However, if gdbserver starts
>> without problems, this means that the ptrace check succeeded and that
>> there are no ptrace restrictions in place.  Therefore, an "attach" would
>> succeed as well.  Which means that even if we're running GDB in a
>> ptrace-restricted system, things will go OK as long as gdbserver is not
>> restricted.  In this case, we wouldn't see any error messages IIUC.
>
> So
>
>  $ gdbserver --multi :9999
>
> exits with error immediately?

Yeah.

> You could start gdbserver with the restrictions off (like a long
> lived daemon), and then while gdbserver is running enable
> restrictions, I suppose.

Ah, right, I think that would also work.

Thanks,

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
On 9/4/19 9:56 PM, Sergio Durigan Junior wrote:
>> You could start gdbserver with the restrictions off (like a long
>> lived daemon), and then while gdbserver is running enable
>> restrictions, I suppose.
> Ah, right, I think that would also work.

I'm interested in knowing whether the error percolates to
the gdb side in a reasonable form.

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

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

Sergio Durigan Junior
On Wednesday, September 04 2019, Pedro Alves wrote:

> On 9/4/19 9:56 PM, Sergio Durigan Junior wrote:
>>> You could start gdbserver with the restrictions off (like a long
>>> lived daemon), and then while gdbserver is running enable
>>> restrictions, I suppose.
>> Ah, right, I think that would also work.
>
> I'm interested in knowing whether the error percolates to
> the gdb side in a reasonable form.

The attach error is displayed by GDB, but the information about ptrace
restrictions is not.

This is what I see on GDB:

  (gdb) attach 32378
  Attaching to process 32378
  Attaching to process 32378 failed

And this is what I see on gdbserver:

  gdbserver: Cannot attach to process 32378: Permission denied (13), the SELinux boolean 'deny_ptrace' is enabled, you can disable this process attach protection by: (gdb) shell sudo setsebool deny_ptrace=0


I think there's something wrong with the terminal settings because
newlines aren't being respected here.  But the message is still there,
and the user can still understand it, I think.

Ideally we'd have GDB also display the ptrace restriction warning, but I
think that if the user can see that the attach failed, she will likely
look at what happened with gdbserver, and will see the error there.  If
we were to show the message on GDB, we'd also have to rewrite it in a
way that explains that the command needs to be run at the target, which
may confuse things.

Thanks,

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
On 9/4/19 10:36 PM, Sergio Durigan Junior wrote:

> On Wednesday, September 04 2019, Pedro Alves wrote:
>
>> On 9/4/19 9:56 PM, Sergio Durigan Junior wrote:
>>>> You could start gdbserver with the restrictions off (like a long
>>>> lived daemon), and then while gdbserver is running enable
>>>> restrictions, I suppose.
>>> Ah, right, I think that would also work.
>>
>> I'm interested in knowing whether the error percolates to
>> the gdb side in a reasonable form.
>
> The attach error is displayed by GDB, but the information about ptrace
> restrictions is not.
>
> This is what I see on GDB:
>
>   (gdb) attach 32378
>   Attaching to process 32378
>   Attaching to process 32378 failed
>
> And this is what I see on gdbserver:
>
>   gdbserver: Cannot attach to process 32378: Permission denied (13), the SELinux boolean 'deny_ptrace' is enabled, you can disable this process attach protection by: (gdb) shell sudo setsebool deny_ptrace=0
>
>
> I think there's something wrong with the terminal settings because
> newlines aren't being respected here.  But the message is still there,
> and the user can still understand it, I think.

Odd.  We should fix that...

But this made me realize something.  Here:

  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 ());
 +

This is code that is run by the fork child.  Recall that we
had introduced trace_start_error_with_name instead of using
perror_with_name / error, because between after fork and before
exec, we can only safely use async-signal-safe functions.

But that linux_ptrace_me_fail_reason call is doing a lot of
non-async-signal safe things...

I think the right approach would be to make the child pass back the errno
value to the parent, and then have the parent, do the dlopens, the
std::string/malloc uses, etc. and print the error messages.

The traditional way to pass data around like this is via a pipe.
darwin-nat.c uses a pipe around fork, see ptrace_fds, though for a
different reason.

> Ideally we'd have GDB also display the ptrace restriction warning, but I
> think that if the user can see that the attach failed, she will likely
> look at what happened with gdbserver, and will see the error there.  

I don't think that assuming that the user has easy/convenient access
to gdbserver's terminal is a great assumption.  Consider IDEs automating
things, or gdbserver really being used as a service in a container, etc.

Note that at least for Yama, you can have ptrace allow PTRACE_ME, while
PTRACE_ATTACH gets refused.  So it's not guaranteed that gdbserver would
exit out early on start up, meaning, I think that handling this
gdbserver + "(gdb) attach" error case isn't being pedantic.

> If
> we were to show the message on GDB, we'd also have to rewrite it in a
> way that explains that the command needs to be run at the target, which
> may confuse things.
Or it may not confuse things?  I don't think that should prevent
improving things.  Surely we can find some way to express the message
without causing confusion.  I'm not sure it does, but if it does,
maybe we can just append a "on the target system" somewhere?

Thanks,
Pedro Alves
Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
In reply to this post by Sergio Durigan Junior
> From: Sergio Durigan Junior <[hidden email]>
> Cc: Pedro Alves <[hidden email]>,
> Eli Zaretskii <[hidden email]>,
> Ruslan Kabatsayev <[hidden email]>,
> Sergio Durigan Junior <[hidden email]>
> Date: Wed,  4 Sep 2019 15:54:08 -0400
>
> Changes since v2:
>
> - Fixed nits pointed by Pedro.  Incorporated his patch to improve the
>   linux_ptrace_attach_fail_reason_* funtions.
>
> - Fixed documentation issues.

OK for the documentation part.

Thanks.
Reply | Threaded
Open this post in threaded view
|

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

Sergio Durigan Junior
In reply to this post by Pedro Alves-7
On Thursday, September 05 2019, Pedro Alves wrote:

> On 9/4/19 10:36 PM, Sergio Durigan Junior wrote:
>> On Wednesday, September 04 2019, Pedro Alves wrote:
>>
>>> On 9/4/19 9:56 PM, Sergio Durigan Junior wrote:
>>>>> You could start gdbserver with the restrictions off (like a long
>>>>> lived daemon), and then while gdbserver is running enable
>>>>> restrictions, I suppose.
>>>> Ah, right, I think that would also work.
>>>
>>> I'm interested in knowing whether the error percolates to
>>> the gdb side in a reasonable form.
>>
>> The attach error is displayed by GDB, but the information about ptrace
>> restrictions is not.
>>
>> This is what I see on GDB:
>>
>>   (gdb) attach 32378
>>   Attaching to process 32378
>>   Attaching to process 32378 failed
>>
>> And this is what I see on gdbserver:
>>
>>   gdbserver: Cannot attach to process 32378: Permission denied (13),
>> the SELinux boolean 'deny_ptrace' is enabled, you can disable this
>> process attach protection by: (gdb) shell sudo setsebool
>> deny_ptrace=0
>>
>>
>> I think there's something wrong with the terminal settings because
>> newlines aren't being respected here.  But the message is still there,
>> and the user can still understand it, I think.
>
> Odd.  We should fix that...

OK, then.

> But this made me realize something.  Here:
>
>   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 ());
>  +
>
> This is code that is run by the fork child.  Recall that we
> had introduced trace_start_error_with_name instead of using
> perror_with_name / error, because between after fork and before
> exec, we can only safely use async-signal-safe functions.

Ah, I didn't remember the exact reason, thanks for refreshing my
memory.

> But that linux_ptrace_me_fail_reason call is doing a lot of
> non-async-signal safe things...

Fair enough.

> I think the right approach would be to make the child pass back the errno
> value to the parent, and then have the parent, do the dlopens, the
> std::string/malloc uses, etc. and print the error messages.
>
> The traditional way to pass data around like this is via a pipe.
> darwin-nat.c uses a pipe around fork, see ptrace_fds, though for a
> different reason.

Funny, at first (when I was trying out the patch) I noticed that this
ptrace call was failing, but thought that, because it is being called
inside the child, it would be good if we could pass errno back to the
parent, as you said.  But then I just decided to go the easy way and
call trace_start_error_with_name directly there.

I'll look into passing the errno back to the parent via pipes, as
suggested.

>> Ideally we'd have GDB also display the ptrace restriction warning, but I
>> think that if the user can see that the attach failed, she will likely
>> look at what happened with gdbserver, and will see the error there.  
>
> I don't think that assuming that the user has easy/convenient access
> to gdbserver's terminal is a great assumption.  Consider IDEs automating
> things, or gdbserver really being used as a service in a container, etc.
>
> Note that at least for Yama, you can have ptrace allow PTRACE_ME, while
> PTRACE_ATTACH gets refused.  So it's not guaranteed that gdbserver would
> exit out early on start up, meaning, I think that handling this
> gdbserver + "(gdb) attach" error case isn't being pedantic.
>
>> If
>> we were to show the message on GDB, we'd also have to rewrite it in a
>> way that explains that the command needs to be run at the target, which
>> may confuse things.
> Or it may not confuse things?  I don't think that should prevent
> improving things.  Surely we can find some way to express the message
> without causing confusion.  I'm not sure it does, but if it does,
> maybe we can just append a "on the target system" somewhere?

"on the target system" can be confusing for newcomers.  But sure, I can
certainly find a way to write the warning in a non-confusing way.  I'm
just thinking about the extra work involved to implement this.  I'll
design something here and send a v4 when ready.

Thanks,

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Reply | Threaded
Open this post in threaded view
|

[PATCH v4] Improve ptrace-error detection on Linux targets

Sergio Durigan Junior
In reply to this post by Sergio Durigan Junior
Changes from v3:

- Implement a better way to check for the ptrace errno inside
  linux_fork_to_function et al.

- Implement error message passing (gdbserver -> GDB) when attach fails
  on gdbserver.

- Extend docs a little bit to mention that the ptrace restrictions
  apply also to gdbserver (very trivial).

- Improve error message to instruct the user to perform the necessary
  actions in the target side when remote debugging is in play.

- Very minor nits.


In Fedora GDB, we carry the following patch:

  https://src.fedoraproject.org/rpms/gdb/blob/8ac06474ff1e2aa4920d14e0666b083eeaca8952/f/gdb-attach-fail-reasons-5of5.patch

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_TRACEME 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.1" and checks if the "deny_ptrace"
    option is enabled.

  - 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 /usr/bin/true
  ...
  Starting program: /usr/bin/true
  warning: Could not trace the inferior process.
  warning: ptrace: Permission denied
  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

  If you are debugging the inferior remotely, the ptrace restriction(s) need
  to be disabled in the target system (e.g., where GDBserver is running).
  During startup program exited with code 127.
  (gdb)

In case "/proc/sys/kernel/yama/ptrace_scope" is > 0:

  # gdb /usr/bin/true
  ...
  Starting program: /usr/bin/true
  warning: Could not trace the inferior process.
  warning: ptrace: Operation not permitted
  The Linux kernel's Yama ptrace scope is in effect, which can prevent
  GDB from using 'ptrace'.  You can disable it by executing (as root):

    echo 0 > /proc/sys/kernel/yama/ptrace_scope

  If you are debugging the inferior remotely, the ptrace restriction(s) need
  to be disabled in the target system (e.g., where GDBserver is running).
  During startup program exited with code 127.
  (gdb)

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 /usr/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 /usr/bin/true
  gdbserver: linux_ptrace_test_ret_to_nx: Cannot PTRACE_TRACEME: Permission denied
  gdbserver: linux_ptrace_test_ret_to_nx: status 256 is not WIFSTOPPED!
  gdbserver: linux_ptrace_test_ret_to_nx: failed to kill child pid 2766868 No such process
  gdbserver: Could not trace the inferior process.
  gdbserver: ptrace: Permission denied
  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

  If you are debugging the inferior remotely, the ptrace restriction(s) need
  to be disabled in the target system (e.g., where GDBserver is running).
  #

(I decided to keep all the other messages, even though I find them a
bit distracting).

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.
  If you are debugging the inferior remotely, the ptrace restriction(s) need
  to be disabled in the target system (e.g., where GDBserver is running).

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.

Notice how, on every message, we instruct the user to "do the right
thing" if gdbserver is being used.  This is because if the user
started gdbserver *before* any ptrace restriction was in place, and
then, for some reason, one or more restrictions get enabled, then the
error message will be displayed both on gdbserver *and* on the
connected GDB.  Since the user will be piloting GDB, it's important to
explicitly say that the ptrace restrictions are enabled in the target,
where gdbserver is running.

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  <[hidden email]>

        * gdb.texinfo (Linux kernel ptrace restrictions): New appendix
        section.

gdb/ChangeLog:
yyyy-mm-dd  Sergio Durigan Junior  <[hidden email]>
            Jan Kratochvil  <[hidden email]>
            Pedro Alves  <[hidden email]>

        * 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 (attach_proc_task_lwp_callback): Call
        'linux_ptrace_attach_fail_reason_lwp'.
        (linux_nat_target::attach): Update call to
        'linux_ptrace_attach_fail_reason'.
        (_initialize_linux_nat): Set 'inf_ptrace_me_fail_reason'.
        * 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",
        "gdbsupport/filestuff.h" and "nat/fork-inferior.h".
        (selinux_ftype): New typedef.
        (linux_ptrace_restricted_fail_reason): New function.
        (linux_ptrace_attach_fail_reason_1): New function.
        (linux_ptrace_attach_fail_reason): Change first argument type
        from 'ptid_t' to 'pid_t'.  Call
        'linux_ptrace_attach_fail_reason_1' and
        'linux_ptrace_restricted_fail_reason'.
        (linux_ptrace_attach_fail_reason_lwp): New function.
        (linux_ptrace_me_fail_reason): New function.
        (errno_pipe): New variable.
        (linux_fork_to_function): Initialize pipe before forking.
        (linux_child_function): Deal with errno-passing from child.
        Handle ptrace error.
        (linux_check_child_ptrace_errno): New function.
        (linux_check_child_ptrace_errno): Call
        'linux_check_child_ptrace_errno'.
        * nat/linux-ptrace.h (linux_ptrace_attach_fail_reason): Update
        prototype.
        (linux_ptrace_attach_fail_reason_lwp): New prototype.
        (linux_ptrace_me_fail_reason): New prototype.
        * remote.c (extended_remote_target::attach): Handle error
        message passed by the server when attach fails.

gdb/gdbserver/ChangeLog:
yyyy-mm-dd  Sergio Durigan Junior  <[hidden email]>
            Pedro Alves  <[hidden email]>

        * linux-low.c (linux_ptrace_fun): Call
        'linux_ptrace_me_fail_reason'.
        (attach_proc_task_lwp_callback): Call
        'linux_ptrace_attach_fail_reason_lwp'.
        (linux_attach): Call 'linux_ptrace_attach_fail_reason'.
        * server.c (handle_v_attach): Use try..catch when calling
        'attach_inferior', and send an error message to the client
        when needed.
        * thread-db.c (attach_thread): Call
        'linux_ptrace_attach_fail_reason_lwp'.
---
 gdb/doc/gdb.texinfo        | 143 +++++++++++++++++++++++++++++
 gdb/gdbserver/linux-low.c  |   9 +-
 gdb/gdbserver/server.c     |  14 ++-
 gdb/gdbserver/thread-db.c  |   2 +-
 gdb/gdbsupport/gdb-dlfcn.c |   4 +-
 gdb/gdbsupport/gdb-dlfcn.h |   7 +-
 gdb/inf-ptrace.c           |  17 +++-
 gdb/inf-ptrace.h           |  10 ++
 gdb/linux-nat.c            |   9 +-
 gdb/nat/fork-inferior.c    |   4 +-
 gdb/nat/fork-inferior.h    |   7 +-
 gdb/nat/linux-ptrace.c     | 183 +++++++++++++++++++++++++++++++++++--
 gdb/nat/linux-ptrace.h     |  27 ++++--
 gdb/remote.c               |  16 +++-
 14 files changed, 418 insertions(+), 34 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 53b7de91e4..ffaedc4ba8 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 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
@@ -44689,6 +44692,146 @@ 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} and
+@code{gdbserver} 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 severely restrict
+the ability to perform these operations, which can make @value{GDBN}
+or @code{gdbserver} 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} and
+@code{gdbserver} properly.
+
+@menu
+* The error message::                   The error message displayed when the
+                                        system prevents @value{GDBN}
+                                        or @code{gdbserver} from using
+                                        @code{ptrace}
+* SELinux's deny_ptrace::               SELinux and the @code{deny_ptrace} option
+* Yama's ptrace_scope::                 Yama and the @code{ptrace_scope} setting
+* Docker and seccomp::                  Docker and the @code{seccomp}
+                                        infrastructure
+@end menu
+
+@node The error message
+@appendixsection The error message
+
+When the system prevents @value{GDBN} or @code{gdbserver} 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'.  You can disable it by executing (as root):
+
+  setsebool deny_ptrace off
+
+If you are debugging the inferior remotely, the instruction(s) above must
+be performed in the target system (e.g., where GDBserver is running).
+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} or
+@code{gdbserver} 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} or @code{gdbserver}.  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..2b3c586125 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");
@@ -1169,7 +1170,7 @@ attach_proc_task_lwp_callback (ptid_t ptid)
       else if (err != 0)
  {
   std::string reason
-    = linux_ptrace_attach_fail_reason_string (ptid, err);
+    = linux_ptrace_attach_fail_reason_lwp (ptid, err);
 
   warning (_("Cannot attach to lwp %d: %s"), lwpid, reason.c_str ());
  }
@@ -1201,8 +1202,8 @@ linux_attach (unsigned long pid)
     {
       remove_process (proc);
 
-      std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
-      error ("Cannot attach to process %ld: %s", pid, reason.c_str ());
+      std::string reason = linux_ptrace_attach_fail_reason (pid, err);
+      error (_("Cannot attach to process %ld: %s"), pid, reason.c_str ());
     }
 
   /* Don't ignore the initial SIGSTOP if we just attached to this
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 127cd3840b..51a7066594 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2913,9 +2913,21 @@ handle_v_attach (char *own_buf)
 {
   client_state &cs = get_client_state ();
   int pid;
+  int ret;
 
   pid = strtol (own_buf + 8, NULL, 16);
-  if (pid != 0 && attach_inferior (pid) == 0)
+
+  try
+    {
+      ret = attach_inferior (pid);
+    }
+  catch (const gdb_exception_error &e)
+    {
+      sprintf (own_buf, "E.%s", e.what ());
+      return 0;
+    }
+
+  if (pid != 0 && ret == 0)
     {
       /* Don't report shared library events after attaching, even if
  some libraries are preloaded.  GDB will always poll the
diff --git a/gdb/gdbserver/thread-db.c b/gdb/gdbserver/thread-db.c
index b2791b0513..cfba05977e 100644
--- a/gdb/gdbserver/thread-db.c
+++ b/gdb/gdbserver/thread-db.c
@@ -225,7 +225,7 @@ attach_thread (const td_thrhandle_t *th_p, td_thrinfo_t *ti_p)
   err = linux_attach_lwp (ptid);
   if (err != 0)
     {
-      std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
+      std::string reason = linux_ptrace_attach_fail_reason_lwp (ptid, err);
 
       warning ("Could not attach to thread %ld (LWP %d): %s",
        (unsigned long) ti_p->ti_tid, ti_p->ti_lid, reason.c_str ());
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..b792af00d1 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -94,6 +94,20 @@ 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 {};
+}
+
+/* See inf-ptrace.h.  */
+
+std::string (*inf_ptrace_me_fail_reason) (int err)
+  = default_inf_ptrace_me_fail_reason;
+
 /* Prepare to be traced.  */
 
 static void
@@ -101,7 +115,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
+   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..22cb55e6dc 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1136,7 +1136,7 @@ attach_proc_task_lwp_callback (ptid_t ptid)
   else
     {
       std::string reason
- = linux_ptrace_attach_fail_reason_string (ptid, err);
+ = linux_ptrace_attach_fail_reason_lwp (ptid, err);
 
       warning (_("Cannot attach to lwp %d: %s"),
        lwpid, reason.c_str ());
@@ -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 355e9bef43..2ead4a4858 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..694f390de0 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -21,6 +21,9 @@
 #include "linux-procfs.h"
 #include "linux-waitpid.h"
 #include "gdbsupport/buffer.h"
+#include "gdbsupport/gdb-dlfcn.h"
+#include "nat/fork-inferior.h"
+#include "gdbsupport/filestuff.h"
 #ifdef HAVE_SYS_PROCFS_H
 #include <sys/procfs.h>
 #endif
@@ -30,11 +33,85 @@
    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 *);
 
-std::string
-linux_ptrace_attach_fail_reason (pid_t pid)
+/* 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.1", true);
+
+  if (handle != nullptr)
+    {
+      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'.  You can disable it by executing (as root):\n\
+\n\
+  setsebool deny_ptrace off\n"));
+    }
+
+  gdb_file_up yama_ptrace_scope
+    = gdb_fopen_cloexec ("/proc/sys/kernel/yama/ptrace_scope", "r");
+
+  if (yama_ptrace_scope != nullptr)
+    {
+      char yama_scope = fgetc (yama_ptrace_scope.get ());
+
+      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'.  You can disable it by executing (as root):\n\
+\n\
+  echo 0 > /proc/sys/kernel/yama/ptrace_scope\n"));
+    }
+
+  if (ret.empty ())
+    {
+      /* It wasn't possible to determine the exact reason for the
+ ptrace error.  Let's just emit a generic error message
+ pointing the user to our documentation, where she can find
+ instructions on how to try to diagnose the problem.  */
+      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.");
+    }
+
+  /* The user may be debugging remotely, so we have to warn that
+     the instructions above should be performed in the target.  */
+  string_appendf (ret,
+  _("\n\
+If you are debugging the inferior remotely, the ptrace restriction(s) must\n\
+be disabled in the target system (e.g., where GDBserver is running)."));
+
+  return ret;
+}
+
+/* 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.  Helper for linux_ptrace_attach_fail_reason and
+   linux_ptrace_attach_fail_reason_lwp.  */
+
+static std::string
+linux_ptrace_attach_fail_reason_1 (pid_t pid)
 {
   pid_t tracerpid = linux_proc_get_tracerpid_nowarn (pid);
   std::string result;
@@ -56,10 +133,24 @@ linux_ptrace_attach_fail_reason (pid_t pid)
 /* See linux-ptrace.h.  */
 
 std::string
-linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err)
+linux_ptrace_attach_fail_reason (pid_t pid, int err)
+{
+  std::string result = linux_ptrace_attach_fail_reason_1 (pid);
+  std::string ptrace_restrict = linux_ptrace_restricted_fail_reason (err);
+
+  if (!ptrace_restrict.empty ())
+    result += "\n" + ptrace_restrict;
+
+  return result;
+}
+
+/* See linux-ptrace.h.  */
+
+std::string
+linux_ptrace_attach_fail_reason_lwp (ptid_t ptid, int err)
 {
   long lwpid = ptid.lwp ();
-  std::string reason = linux_ptrace_attach_fail_reason (lwpid);
+  std::string reason = linux_ptrace_attach_fail_reason_1 (lwpid);
 
   if (!reason.empty ())
     return string_printf ("%s (%d), %s", safe_strerror (err), err,
@@ -68,6 +159,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.  */
@@ -257,6 +356,12 @@ linux_ptrace_test_ret_to_nx (void)
 #endif /* defined __i386__ || defined __x86_64__ */
 }
 
+/* If the PTRACE_TRACEME call on linux_child_function errors, we need
+   to be able to send ERRNO back to the parent so that it can check
+   whether there are restrictions in place preventing ptrace from
+   working.  We do that with a pipe.  */
+static int errno_pipe[2];
+
 /* Helper function to fork a process and make the child process call
    the function FUNCTION, passing CHILD_STACK as parameter.
 
@@ -273,6 +378,11 @@ linux_fork_to_function (gdb_byte *child_stack, int (*function) (void *))
   /* Sanity check the function pointer.  */
   gdb_assert (function != NULL);
 
+  /* Create the pipe that will be used by the child to pass ERRNO
+     after the PTRACE_TRACEME call.  */
+  if (pipe (errno_pipe) != 0)
+    trace_start_error_with_name ("pipe");
+
 #if defined(__UCLIBC__) && defined(HAS_NOMMU)
 #define STACK_SIZE 4096
 
@@ -321,7 +431,21 @@ 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);
+  /* Close read end.  */
+  close (errno_pipe[0]);
+
+  int ret = ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0,
+    (PTRACE_TYPE_ARG4) 0);
+  int ptrace_errno = errno;
+
+  /* Write ERRNO to the pipe, even if it's zero, and close the writing
+     end of the pipe.  */
+  write (errno_pipe[1], &ptrace_errno, sizeof (ptrace_errno));
+  close (errno_pipe[1]);
+
+  if (ret != 0)
+    _exit (0);
+
   kill (getpid (), SIGSTOP);
 
   /* Fork a grandchild.  */
@@ -336,6 +460,48 @@ static void linux_test_for_tracesysgood (int child_pid);
 static void linux_test_for_tracefork (int child_pid);
 static void linux_test_for_exitkill (int child_pid);
 
+/* Helper function to wait for the child to send us the ptrace ERRNO,
+   and check if it's OK.  */
+
+static void
+linux_check_child_ptrace_errno ()
+{
+  int child_errno;
+  fd_set rset;
+  struct timeval timeout;
+
+  /* Close the writing end of the pipe.  */
+  close (errno_pipe[1]);
+
+  FD_ZERO (&rset);
+  FD_SET (errno_pipe[0], &rset);
+
+  /* One second should be plenty of time to wait for the child's
+     reply.  */
+  timeout.tv_sec = 1;
+  timeout.tv_usec = 0;
+
+  int ret = select (errno_pipe[0] + 1, &rset, NULL, NULL, &timeout);
+
+  if (ret < 0)
+    trace_start_error_with_name ("select");
+  else if (ret == 0)
+    error (_("Timeout while waiting for child's ptrace errno"));
+  else
+    read (errno_pipe[0], &child_errno, sizeof (child_errno));
+
+  if (child_errno != 0)
+    {
+      /* The child can't use PTRACE_TRACEME.  We just bail out.  */
+      std::string reason = linux_ptrace_restricted_fail_reason (child_errno);
+
+      errno = child_errno;
+      trace_start_error_with_name ("ptrace", reason.c_str ());
+    }
+
+  close (errno_pipe[0]);
+}
+
 /* Determine ptrace features available on this target.  */
 
 void
@@ -352,6 +518,9 @@ linux_check_ptrace_features (void)
      reporting.  */
   child_pid = linux_fork_to_function (NULL, linux_child_function);
 
+  /* Check if the child can successfully use ptrace.  */
+  linux_check_child_ptrace_errno ();
+
   ret = my_waitpid (child_pid, &status, 0);
   if (ret == -1)
     perror_with_name (("waitpid"));
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index fd2f12a342..90afb60f34 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -176,12 +176,27 @@ struct buffer;
 # define TRAP_HWBKPT 4
 #endif
 
-extern std::string linux_ptrace_attach_fail_reason (pid_t pid);
-
-/* 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);
+/* 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);
+
+/* Find all possible reasons we could have failed to attach to PID's
+   LWPID and return them as a string.  ERR is the error PTRACE_ATTACH
+   failed with (an errno).  Unlike linux_ptrace_attach_fail_reason,
+   this function should be used when attaching to an LWP other than
+   the leader; it does not warn about ptrace restrictions.  */
+extern std::string linux_ptrace_attach_fail_reason_lwp (ptid_t pid, 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);
diff --git a/gdb/remote.c b/gdb/remote.c
index ae06c4ba79..e5780bf586 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5831,8 +5831,20 @@ extended_remote_target::attach (const char *args, int from_tty)
     case PACKET_UNKNOWN:
       error (_("This target does not support attaching to a process"));
     default:
-      error (_("Attaching to %s failed"),
-     target_pid_to_str (ptid_t (pid)).c_str ());
+      {
+ std::string errmsg = rs->buf.data ();
+
+ if (!errmsg.empty ())
+  {
+    /* Get rid of the "E." prefix.  */
+    errmsg.erase (0, 2);
+  }
+
+ error (_("Attaching to %s failed%s%s"),
+       target_pid_to_str (ptid_t (pid)).c_str (),
+       !errmsg.empty () ? "\n" : "",
+       errmsg.c_str ());
+      }
     }
 
   set_current_inferior (remote_add_inferior (false, pid, 1, 0));
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: Sergio Durigan Junior <[hidden email]>
> Cc: Pedro Alves <[hidden email]>,
> Eli Zaretskii <[hidden email]>,
> Ruslan Kabatsayev <[hidden email]>,
> Sergio Durigan Junior <[hidden email]>
> Date: Tue, 10 Sep 2019 21:11:03 -0400
>
> +The SELinux 'deny_ptrace' option is enabled and preventing GDB
                                                              ^^^
@value{GDBN}

Other than that, the documentation part is OK.

Thanks.
Reply | Threaded
Open this post in threaded view
|

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

Sergio Durigan Junior
On Thursday, September 12 2019, Eli Zaretskii wrote:

>> From: Sergio Durigan Junior <[hidden email]>
>> Cc: Pedro Alves <[hidden email]>,
>> Eli Zaretskii <[hidden email]>,
>> Ruslan Kabatsayev <[hidden email]>,
>> Sergio Durigan Junior <[hidden email]>
>> Date: Tue, 10 Sep 2019 21:11:03 -0400
>>
>> +The SELinux 'deny_ptrace' option is enabled and preventing GDB
>                                                               ^^^
> @value{GDBN}
>
> Other than that, the documentation part is OK.

Thanks, fixed.

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
12345