[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
|

[PATCH] Improve ptrace-error detection on Linux targets

Sergio Durigan Junior
In Fedora GDB, we carry the following patch:

  https://src.fedoraproject.org/rpms/gdb/blob/master/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.

Instead of printing a very certain warning about a very specific
setting, I decided to propose the following patch, which prints a
generic warning about a possible situation (i.e., without going
bothering to make sure that the situation is actually happening).
What this means is that whenever a ptrace error is detected, and if
errno is either EACCES or EPERM, a warning will be printed pointing
the user to our documentation, where she will find more details about
possible restrictions in place against ptrace.

The patch obviously 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.

  - YAMA's /proc/sys/kernel/yama/ptrace_scope setting.

  - seccomp on Docker containers.

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.

For the PTRACE_ATTACH scenario, I borrowed the original patch's idea
and hacked the warning into 'linux_ptrace_attach_fail_reason'.  For
PTRACE_ME, I opted to implement a new target method called
'ptrace_me_fail_reason' which is then be called inside inf-ptrace.c's
'inf_ptrace_me'.  This method is currently only implemented by the
Linux target.

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.  Maybe something like using
dlopen (RTLD_NEXT... and wrapping ptrace could work, but I don't know
offhand.

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]>

        * inf-ptrace.c (inf_ptrace_me): Update call to
        'trace_start_error_with_name'.
        * linux-nat.c (linux_nat_target::ptrace_me_fail_reason): New
        method.
        (linux_nat_target::attach): Update call to
        'linux_ptrace_attach_fail_reason'.
        * linux-nat.h (linux_nat_target) <ptrace_me_fail_reason>: New
        method.
        * nat/fork-inferior.c (trace_start_error_with_name): Add
        optional 'append' argument.
        * nat/fork-inferior.h (trace_start_error_with_name): Update
        prototype.
        * nat/linux-ptrace.c (linux_ptrace_attach_fail_reason): Add
        optional 'err' argument.
        * nat/linux-ptrace.h (linux_ptrace_attach_fail_reason): Update
        prototype.
        * target-delegates.c: Regenerate.
        * target.h (struct target_ops) <ptrace_me_fail_reason>: New
        method.
        (target_ptrace_me_fail_reason): New define.
---
 gdb/doc/gdb.texinfo     | 90 +++++++++++++++++++++++++++++++++++++++++
 gdb/inf-ptrace.c        |  3 +-
 gdb/linux-nat.c         | 19 ++++++++-
 gdb/linux-nat.h         |  2 +
 gdb/nat/fork-inferior.c |  4 +-
 gdb/nat/fork-inferior.h |  7 ++--
 gdb/nat/linux-ptrace.c  | 11 +++--
 gdb/nat/linux-ptrace.h  |  6 ++-
 gdb/target-delegates.c  | 28 +++++++++++++
 gdb/target.h            | 15 +++++++
 10 files changed, 174 insertions(+), 11 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bcf0420779..d2aab9be45 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -182,6 +182,9 @@ software in general.  We will miss him.
                                 @value{GDBN}
 * Operating System Information:: Getting additional information from
                                  the operating system
+* Linux kernel @code{ptrace} restrictions:: Restrictions sometimes
+                                            imposed by the Linux
+                                            kernel on @code{ptrace}
 * Trace File Format:: GDB trace file format
 * Index Section Format::        .gdb_index section format
 * Man Pages:: Manual pages
@@ -44672,6 +44675,93 @@ 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 @code{ptrace} restrictions
+@appendix Linux kernel @code{ptrace} restrictions
+@cindex linux kernel ptrace restrictions, attach
+
+The @code{ptrace} system call is used by @value{GDBN} to, among other
+things, attach to a new or existing inferior in order to start
+debugging it.  Due to security concerns, some distributions and
+vendors disable or severily restrict the ability to perform these
+operations, which can make @value{GDBN} malfunction.  In this section,
+we will expand on how this malfunction can manifest, and how to modify
+the system's settings in order to be able to use @value{GDBN}
+properly.
+
+@menu
+* SELinux's @code{deny_ptrace}::        SELinux and the @code{deny_ptrace} option
+* Yama's @code{ptrace_scope}::          Yama and the @code{ptrace_scope} setting
+* Docker and @code{seccomp}::           Docker and the @code{seccomp}
+                                        infrastructure
+@end menu
+
+@node SELinux's @code{deny_ptrace}
+@appendixsection SELinux's @code{deny_ptrace}
+@cindex selinux, 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:
+
+@smallexample
+# setsebool -P deny_ptrace off
+@end smallexample
+
+@node Yama's @code{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:
+
+@smallexample
+# sysctl kernel.yama.ptrace_scope=0
+kernel.yama.ptrace_scope = 0
+@end smallexample
+
+You can make this permanent by doing:
+
+@smallexample
+# sysctl -w kernel.yama.ptrace_scope=0
+kernel.yama.ptrace_scope = 0
+@end smallexample
+
+@node Docker and @code{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/inf-ptrace.c b/gdb/inf-ptrace.c
index 4a8e732373..4e21f677a2 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -101,7 +101,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",
+ target_ptrace_me_fail_reason (errno).c_str ());
 }
 
 /* Start a new inferior Unix child process.  EXEC_FILE is the file to
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 945c19f666..053d7630b8 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -614,6 +614,22 @@ linux_nat_target::follow_fork (int follow_child, int detach_fork)
   return 0;
 }
 
+/* See the declaration on target.h.  */
+
+std::string
+linux_nat_target::ptrace_me_fail_reason (int err)
+{
+  std::string ret;
+
+  if (err == EACCES || err == EPERM)
+    ret = _("There might be restrictions preventing ptrace from\n"
+    "working.  Please see the appendix "
+    "\"Linux kernel ptrace restrictions\"\n"
+    "in the GDB documentation for more details.");
+
+  return ret;
+}
+
 
 int
 linux_nat_target::insert_fork_catchpoint (int pid)
@@ -1191,8 +1207,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 (),
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 0c1695ad10..1aa9b50130 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -134,6 +134,8 @@ public:
 
   int follow_fork (int, int) override;
 
+  std::string ptrace_me_fail_reason (int err) override;
+
   std::vector<static_tracepoint_marker>
     static_tracepoint_markers_by_strid (const char *id) override;
 
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 68b51aa814..72ac623e20 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -591,7 +591,7 @@ trace_start_error (const char *fmt, ...)
 /* See nat/fork-inferior.h.  */
 
 void
-trace_start_error_with_name (const char *string)
+trace_start_error_with_name (const char *string, const char *append)
 {
-  trace_start_error ("%s: %s", string, safe_strerror (errno));
+  trace_start_error ("%s: %s%s", string, safe_strerror (errno), append);
 }
diff --git a/gdb/nat/fork-inferior.h b/gdb/nat/fork-inferior.h
index 1d0519fb26..7e6b889210 100644
--- a/gdb/nat/fork-inferior.h
+++ b/gdb/nat/fork-inferior.h
@@ -98,9 +98,10 @@ extern void trace_start_error (const char *fmt, ...)
   ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
 
 /* Like "trace_start_error", but the error message is constructed by
-   combining STRING with the system error message for errno.  This
-   function does not return.  */
-extern void trace_start_error_with_name (const char *string)
+   combining STRING with the system error message for errno, and
+   (optionally) with APPEND.  This function does not return.  */
+extern void trace_start_error_with_name (const char *string,
+ const char *append = "")
   ATTRIBUTE_NORETURN;
 
 #endif /* NAT_FORK_INFERIOR_H */
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index c1ebc0a032..fa5584829e 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -30,11 +30,10 @@
    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.  */
+/* See declaration in linux-ptrace.h.  */
 
 std::string
-linux_ptrace_attach_fail_reason (pid_t pid)
+linux_ptrace_attach_fail_reason (pid_t pid, int err)
 {
   pid_t tracerpid = linux_proc_get_tracerpid_nowarn (pid);
   std::string result;
@@ -50,6 +49,12 @@ linux_ptrace_attach_fail_reason (pid_t pid)
       "terminated"),
     (int) pid);
 
+  if (err == EACCES || err == EPERM)
+    string_appendf (result,
+    _("There might be restrictions preventing ptrace from\n"
+      "working.  Please see the appendix "
+      "\"Linux kernel ptrace restrictions\"\n"
+      "in the GDB documentation for more details."));
   return result;
 }
 
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index fd2f12a342..99a7780ac7 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -176,7 +176,11 @@ 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);
 
 /* 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
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 52034fe436..7fe45412cb 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -52,6 +52,7 @@ struct dummy_target : public target_ops
   void kill () override;
   void load (const char *arg0, int arg1) override;
   void post_startup_inferior (ptid_t arg0) override;
+  std::string ptrace_me_fail_reason (int arg0) override;
   int insert_fork_catchpoint (int arg0) override;
   int remove_fork_catchpoint (int arg0) override;
   int insert_vfork_catchpoint (int arg0) override;
@@ -220,6 +221,7 @@ struct debug_target : public target_ops
   void kill () override;
   void load (const char *arg0, int arg1) override;
   void post_startup_inferior (ptid_t arg0) override;
+  std::string ptrace_me_fail_reason (int arg0) override;
   int insert_fork_catchpoint (int arg0) override;
   int remove_fork_catchpoint (int arg0) override;
   int insert_vfork_catchpoint (int arg0) override;
@@ -1400,6 +1402,32 @@ debug_target::post_startup_inferior (ptid_t arg0)
   fputs_unfiltered (")\n", gdb_stdlog);
 }
 
+std::string
+target_ops::ptrace_me_fail_reason (int arg0)
+{
+  return this->beneath ()->ptrace_me_fail_reason (arg0);
+}
+
+std::string
+dummy_target::ptrace_me_fail_reason (int arg0)
+{
+  return std::string ();
+}
+
+std::string
+debug_target::ptrace_me_fail_reason (int arg0)
+{
+  std::string result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->ptrace_me_fail_reason (...)\n", this->beneath ()->shortname ());
+  result = this->beneath ()->ptrace_me_fail_reason (arg0);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->ptrace_me_fail_reason (", this->beneath ()->shortname ());
+  target_debug_print_int (arg0);
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_std_string (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
 int
 target_ops::insert_fork_catchpoint (int arg0)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 4e2e75cb80..8e6cc75d8e 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -608,6 +608,18 @@ struct target_ops
   char **, int);
     virtual void post_startup_inferior (ptid_t)
       TARGET_DEFAULT_IGNORE ();
+    /* When the call to ptrace fails, and we have already forked, this
+       function can be called in order to try to obtain the reason why
+       ptrace failed.  It takes one integer argument, which should be
+       the ERRNO value returned by ptrace.
+
+       This function will return a 'std::string' containing the fail
+       reason, or an empty string otherwise.
+
+       The reason this is a target method is because different targets
+       can compute the reason in different ways.  */
+    virtual std::string ptrace_me_fail_reason (int)
+      TARGET_DEFAULT_RETURN (std::string ());
     virtual int insert_fork_catchpoint (int)
       TARGET_DEFAULT_RETURN (1);
     virtual int remove_fork_catchpoint (int)
@@ -1624,6 +1636,9 @@ extern void target_load (const char *arg, int from_tty);
 #define target_remove_vfork_catchpoint(pid) \
      (current_top_target ()->remove_vfork_catchpoint) (pid)
 
+#define target_ptrace_me_fail_reason(err) \
+     (current_top_target ()->ptrace_me_fail_reason) (err)
+
 /* If the inferior forks or vforks, this function will be called at
    the next resume in order to perform any bookkeeping and fiddling
    necessary to continue debugging either the parent or child, as
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

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

Ruslan Kabatsayev
Hello Sergio,

On Mon, 19 Aug 2019 at 06:29, Sergio Durigan Junior <[hidden email]> wrote:

>
> In Fedora GDB, we carry the following patch:
>
>   https://src.fedoraproject.org/rpms/gdb/blob/master/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.
>
> Instead of printing a very certain warning about a very specific
> setting, I decided to propose the following patch, which prints a
> generic warning about a possible situation (i.e., without going
> bothering to make sure that the situation is actually happening).
> What this means is that whenever a ptrace error is detected, and if
> errno is either EACCES or EPERM, a warning will be printed pointing
> the user to our documentation, where she will find more details about
> possible restrictions in place against ptrace.

Doesn't it worsen user experience compared to the current Fedora
patch? Now a user first coming across the problem will have to
navigate the documentation, then check possible scenarios – instead of
simply getting a friendly explanation what exactly is wrong.

Compare this with the patch applied by some Linux distributions
(CentOS IIRC) to make Bash emit

> bash: myprogram: /lib/ld-linux.so.2: bad ELF interpreter: No such file or directory

when ELF interpreter can't be found. By default, Bash only prints the
less than helpful message

> bash: myprogram: No such file or directory

Your proposal (if it's intended to replace Fedora's GDB patch) is
similar to replacing this default with

> bash: myprogram: No such file or directory. There might be other reasons than simply command executable not existing. Please see Bash documentation section XYZ for details.

and adding a section XYZ in Bash docs explaining that the reasons
could include lacking libraries, broken symlink to the binary, stale
hash table of PATH lookups, etc.. This is considerably worse than the
patch with explicit announcement of actual problem.

So as a user, I disfavor this approach, although it might be more
maintainable for GDB developers. But, if dependency on libselinux is
the only objection, then why not simply dlopen it as needed (and
gracefully handle failure to do so) instead of ELF-level linking to
it? Then there'll be no hard dependency on libselinux, while user
experience will not degrade too.

Regards,
Ruslan

>
> The patch obviously 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.
>
>   - YAMA's /proc/sys/kernel/yama/ptrace_scope setting.
>
>   - seccomp on Docker containers.
>
> 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.
>
> For the PTRACE_ATTACH scenario, I borrowed the original patch's idea
> and hacked the warning into 'linux_ptrace_attach_fail_reason'.  For
> PTRACE_ME, I opted to implement a new target method called
> 'ptrace_me_fail_reason' which is then be called inside inf-ptrace.c's
> 'inf_ptrace_me'.  This method is currently only implemented by the
> Linux target.
>
> 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.  Maybe something like using
> dlopen (RTLD_NEXT... and wrapping ptrace could work, but I don't know
> offhand.
>
> 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]>
>
>         * inf-ptrace.c (inf_ptrace_me): Update call to
>         'trace_start_error_with_name'.
>         * linux-nat.c (linux_nat_target::ptrace_me_fail_reason): New
>         method.
>         (linux_nat_target::attach): Update call to
>         'linux_ptrace_attach_fail_reason'.
>         * linux-nat.h (linux_nat_target) <ptrace_me_fail_reason>: New
>         method.
>         * nat/fork-inferior.c (trace_start_error_with_name): Add
>         optional 'append' argument.
>         * nat/fork-inferior.h (trace_start_error_with_name): Update
>         prototype.
>         * nat/linux-ptrace.c (linux_ptrace_attach_fail_reason): Add
>         optional 'err' argument.
>         * nat/linux-ptrace.h (linux_ptrace_attach_fail_reason): Update
>         prototype.
>         * target-delegates.c: Regenerate.
>         * target.h (struct target_ops) <ptrace_me_fail_reason>: New
>         method.
>         (target_ptrace_me_fail_reason): New define.
> ---
>  gdb/doc/gdb.texinfo     | 90 +++++++++++++++++++++++++++++++++++++++++
>  gdb/inf-ptrace.c        |  3 +-
>  gdb/linux-nat.c         | 19 ++++++++-
>  gdb/linux-nat.h         |  2 +
>  gdb/nat/fork-inferior.c |  4 +-
>  gdb/nat/fork-inferior.h |  7 ++--
>  gdb/nat/linux-ptrace.c  | 11 +++--
>  gdb/nat/linux-ptrace.h  |  6 ++-
>  gdb/target-delegates.c  | 28 +++++++++++++
>  gdb/target.h            | 15 +++++++
>  10 files changed, 174 insertions(+), 11 deletions(-)
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index bcf0420779..d2aab9be45 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -182,6 +182,9 @@ software in general.  We will miss him.
>                                  @value{GDBN}
>  * Operating System Information:: Getting additional information from
>                                   the operating system
> +* Linux kernel @code{ptrace} restrictions:: Restrictions sometimes
> +                                            imposed by the Linux
> +                                            kernel on @code{ptrace}
>  * Trace File Format::          GDB trace file format
>  * Index Section Format::        .gdb_index section format
>  * Man Pages::                  Manual pages
> @@ -44672,6 +44675,93 @@ 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 @code{ptrace} restrictions
> +@appendix Linux kernel @code{ptrace} restrictions
> +@cindex linux kernel ptrace restrictions, attach
> +
> +The @code{ptrace} system call is used by @value{GDBN} to, among other
> +things, attach to a new or existing inferior in order to start
> +debugging it.  Due to security concerns, some distributions and
> +vendors disable or severily restrict the ability to perform these
> +operations, which can make @value{GDBN} malfunction.  In this section,
> +we will expand on how this malfunction can manifest, and how to modify
> +the system's settings in order to be able to use @value{GDBN}
> +properly.
> +
> +@menu
> +* SELinux's @code{deny_ptrace}::        SELinux and the @code{deny_ptrace} option
> +* Yama's @code{ptrace_scope}::          Yama and the @code{ptrace_scope} setting
> +* Docker and @code{seccomp}::           Docker and the @code{seccomp}
> +                                        infrastructure
> +@end menu
> +
> +@node SELinux's @code{deny_ptrace}
> +@appendixsection SELinux's @code{deny_ptrace}
> +@cindex selinux, 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:
> +
> +@smallexample
> +# setsebool -P deny_ptrace off
> +@end smallexample
> +
> +@node Yama's @code{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:
> +
> +@smallexample
> +# sysctl kernel.yama.ptrace_scope=0
> +kernel.yama.ptrace_scope = 0
> +@end smallexample
> +
> +You can make this permanent by doing:
> +
> +@smallexample
> +# sysctl -w kernel.yama.ptrace_scope=0
> +kernel.yama.ptrace_scope = 0
> +@end smallexample
> +
> +@node Docker and @code{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/inf-ptrace.c b/gdb/inf-ptrace.c
> index 4a8e732373..4e21f677a2 100644
> --- a/gdb/inf-ptrace.c
> +++ b/gdb/inf-ptrace.c
> @@ -101,7 +101,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",
> +                                target_ptrace_me_fail_reason (errno).c_str ());
>  }
>
>  /* Start a new inferior Unix child process.  EXEC_FILE is the file to
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 945c19f666..053d7630b8 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -614,6 +614,22 @@ linux_nat_target::follow_fork (int follow_child, int detach_fork)
>    return 0;
>  }
>
> +/* See the declaration on target.h.  */
> +
> +std::string
> +linux_nat_target::ptrace_me_fail_reason (int err)
> +{
> +  std::string ret;
> +
> +  if (err == EACCES || err == EPERM)
> +    ret = _("There might be restrictions preventing ptrace from\n"
> +           "working.  Please see the appendix "
> +           "\"Linux kernel ptrace restrictions\"\n"
> +           "in the GDB documentation for more details.");
> +
> +  return ret;
> +}
> +
>
>  int
>  linux_nat_target::insert_fork_catchpoint (int pid)
> @@ -1191,8 +1207,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 (),
> diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
> index 0c1695ad10..1aa9b50130 100644
> --- a/gdb/linux-nat.h
> +++ b/gdb/linux-nat.h
> @@ -134,6 +134,8 @@ public:
>
>    int follow_fork (int, int) override;
>
> +  std::string ptrace_me_fail_reason (int err) override;
> +
>    std::vector<static_tracepoint_marker>
>      static_tracepoint_markers_by_strid (const char *id) override;
>
> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
> index 68b51aa814..72ac623e20 100644
> --- a/gdb/nat/fork-inferior.c
> +++ b/gdb/nat/fork-inferior.c
> @@ -591,7 +591,7 @@ trace_start_error (const char *fmt, ...)
>  /* See nat/fork-inferior.h.  */
>
>  void
> -trace_start_error_with_name (const char *string)
> +trace_start_error_with_name (const char *string, const char *append)
>  {
> -  trace_start_error ("%s: %s", string, safe_strerror (errno));
> +  trace_start_error ("%s: %s%s", string, safe_strerror (errno), append);
>  }
> diff --git a/gdb/nat/fork-inferior.h b/gdb/nat/fork-inferior.h
> index 1d0519fb26..7e6b889210 100644
> --- a/gdb/nat/fork-inferior.h
> +++ b/gdb/nat/fork-inferior.h
> @@ -98,9 +98,10 @@ extern void trace_start_error (const char *fmt, ...)
>    ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
>
>  /* Like "trace_start_error", but the error message is constructed by
> -   combining STRING with the system error message for errno.  This
> -   function does not return.  */
> -extern void trace_start_error_with_name (const char *string)
> +   combining STRING with the system error message for errno, and
> +   (optionally) with APPEND.  This function does not return.  */
> +extern void trace_start_error_with_name (const char *string,
> +                                        const char *append = "")
>    ATTRIBUTE_NORETURN;
>
>  #endif /* NAT_FORK_INFERIOR_H */
> diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
> index c1ebc0a032..fa5584829e 100644
> --- a/gdb/nat/linux-ptrace.c
> +++ b/gdb/nat/linux-ptrace.c
> @@ -30,11 +30,10 @@
>     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.  */
> +/* See declaration in linux-ptrace.h.  */
>
>  std::string
> -linux_ptrace_attach_fail_reason (pid_t pid)
> +linux_ptrace_attach_fail_reason (pid_t pid, int err)
>  {
>    pid_t tracerpid = linux_proc_get_tracerpid_nowarn (pid);
>    std::string result;
> @@ -50,6 +49,12 @@ linux_ptrace_attach_fail_reason (pid_t pid)
>                       "terminated"),
>                     (int) pid);
>
> +  if (err == EACCES || err == EPERM)
> +    string_appendf (result,
> +                   _("There might be restrictions preventing ptrace from\n"
> +                     "working.  Please see the appendix "
> +                     "\"Linux kernel ptrace restrictions\"\n"
> +                     "in the GDB documentation for more details."));
>    return result;
>  }
>
> diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
> index fd2f12a342..99a7780ac7 100644
> --- a/gdb/nat/linux-ptrace.h
> +++ b/gdb/nat/linux-ptrace.h
> @@ -176,7 +176,11 @@ 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);
>
>  /* 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
> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
> index 52034fe436..7fe45412cb 100644
> --- a/gdb/target-delegates.c
> +++ b/gdb/target-delegates.c
> @@ -52,6 +52,7 @@ struct dummy_target : public target_ops
>    void kill () override;
>    void load (const char *arg0, int arg1) override;
>    void post_startup_inferior (ptid_t arg0) override;
> +  std::string ptrace_me_fail_reason (int arg0) override;
>    int insert_fork_catchpoint (int arg0) override;
>    int remove_fork_catchpoint (int arg0) override;
>    int insert_vfork_catchpoint (int arg0) override;
> @@ -220,6 +221,7 @@ struct debug_target : public target_ops
>    void kill () override;
>    void load (const char *arg0, int arg1) override;
>    void post_startup_inferior (ptid_t arg0) override;
> +  std::string ptrace_me_fail_reason (int arg0) override;
>    int insert_fork_catchpoint (int arg0) override;
>    int remove_fork_catchpoint (int arg0) override;
>    int insert_vfork_catchpoint (int arg0) override;
> @@ -1400,6 +1402,32 @@ debug_target::post_startup_inferior (ptid_t arg0)
>    fputs_unfiltered (")\n", gdb_stdlog);
>  }
>
> +std::string
> +target_ops::ptrace_me_fail_reason (int arg0)
> +{
> +  return this->beneath ()->ptrace_me_fail_reason (arg0);
> +}
> +
> +std::string
> +dummy_target::ptrace_me_fail_reason (int arg0)
> +{
> +  return std::string ();
> +}
> +
> +std::string
> +debug_target::ptrace_me_fail_reason (int arg0)
> +{
> +  std::string result;
> +  fprintf_unfiltered (gdb_stdlog, "-> %s->ptrace_me_fail_reason (...)\n", this->beneath ()->shortname ());
> +  result = this->beneath ()->ptrace_me_fail_reason (arg0);
> +  fprintf_unfiltered (gdb_stdlog, "<- %s->ptrace_me_fail_reason (", this->beneath ()->shortname ());
> +  target_debug_print_int (arg0);
> +  fputs_unfiltered (") = ", gdb_stdlog);
> +  target_debug_print_std_string (result);
> +  fputs_unfiltered ("\n", gdb_stdlog);
> +  return result;
> +}
> +
>  int
>  target_ops::insert_fork_catchpoint (int arg0)
>  {
> diff --git a/gdb/target.h b/gdb/target.h
> index 4e2e75cb80..8e6cc75d8e 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -608,6 +608,18 @@ struct target_ops
>                                   char **, int);
>      virtual void post_startup_inferior (ptid_t)
>        TARGET_DEFAULT_IGNORE ();
> +    /* When the call to ptrace fails, and we have already forked, this
> +       function can be called in order to try to obtain the reason why
> +       ptrace failed.  It takes one integer argument, which should be
> +       the ERRNO value returned by ptrace.
> +
> +       This function will return a 'std::string' containing the fail
> +       reason, or an empty string otherwise.
> +
> +       The reason this is a target method is because different targets
> +       can compute the reason in different ways.  */
> +    virtual std::string ptrace_me_fail_reason (int)
> +      TARGET_DEFAULT_RETURN (std::string ());
>      virtual int insert_fork_catchpoint (int)
>        TARGET_DEFAULT_RETURN (1);
>      virtual int remove_fork_catchpoint (int)
> @@ -1624,6 +1636,9 @@ extern void target_load (const char *arg, int from_tty);
>  #define target_remove_vfork_catchpoint(pid) \
>       (current_top_target ()->remove_vfork_catchpoint) (pid)
>
> +#define target_ptrace_me_fail_reason(err) \
> +     (current_top_target ()->ptrace_me_fail_reason) (err)
> +
>  /* If the inferior forks or vforks, this function will be called at
>     the next resume in order to perform any bookkeeping and fiddling
>     necessary to continue debugging either the parent or child, as
> --
> 2.21.0
>
Reply | Threaded
Open this post in threaded view
|

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

Sergio Durigan Junior
On Monday, August 19 2019, Ruslan Kabatsayev wrote:

> Hello Sergio,

Hey Ruslan,

> On Mon, 19 Aug 2019 at 06:29, Sergio Durigan Junior <[hidden email]> wrote:
>>
>> In Fedora GDB, we carry the following patch:
>>
>>   https://src.fedoraproject.org/rpms/gdb/blob/master/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.
>>
>> Instead of printing a very certain warning about a very specific
>> setting, I decided to propose the following patch, which prints a
>> generic warning about a possible situation (i.e., without going
>> bothering to make sure that the situation is actually happening).
>> What this means is that whenever a ptrace error is detected, and if
>> errno is either EACCES or EPERM, a warning will be printed pointing
>> the user to our documentation, where she will find more details about
>> possible restrictions in place against ptrace.
>
> Doesn't it worsen user experience compared to the current Fedora
> patch? Now a user first coming across the problem will have to
> navigate the documentation, then check possible scenarios – instead of
> simply getting a friendly explanation what exactly is wrong.

I agree that it is easier to just print the reason for the failure
directly, instead of pointing to the documentation.  However, the reason
I chose the latter is because it may not always be possible to identify
the reason, and this might confuse the user or lead him to wrong
conclusions.  I'm mostly thinking about the seccomp feature here
(because I have no idea how to check if it's active or not), but I'm
also concerned that it's unreasonable to expect users to have
libselinux-devel (on Fedora) installed.

> Compare this with the patch applied by some Linux distributions
> (CentOS IIRC) to make Bash emit
>
>> bash: myprogram: /lib/ld-linux.so.2: bad ELF interpreter: No such file or directory
>
> when ELF interpreter can't be found. By default, Bash only prints the
> less than helpful message
>
>> bash: myprogram: No such file or directory
>
> Your proposal (if it's intended to replace Fedora's GDB patch) is
> similar to replacing this default with
>
>> bash: myprogram: No such file or directory. There might be other reasons than simply command executable not existing. Please see Bash documentation section XYZ for details.
>
> and adding a section XYZ in Bash docs explaining that the reasons
> could include lacking libraries, broken symlink to the binary, stale
> hash table of PATH lookups, etc.. This is considerably worse than the
> patch with explicit announcement of actual problem.

Well, in this specific case it's probably not difficult to check for the
right cause and print the specific warning.  However, if there were more
than 2 or 3 scenarios where a program couldn't be executed, and if
checking for them involved depending on external libraries and such,
then we'd be talking about a similar situation here.

> So as a user, I disfavor this approach, although it might be more
> maintainable for GDB developers. But, if dependency on libselinux is
> the only objection, then why not simply dlopen it as needed (and
> gracefully handle failure to do so) instead of ELF-level linking to
> it? Then there'll be no hard dependency on libselinux, while user
> experience will not degrade too.

Thanks for expressing your opinion as user, much appreciated!

I had considered dlopen'ing libselinux before, but didn't do that
because of what I mentioned earlier: I think it's not reasonable to
expect that the user will have libselinux-devel installed, and SELinux
is just one of the possible scenarios here.

But your message made me rethink and decide to do the following:

- Try to dlopen libselinux, and check for deny_ptrace if possible.  If
  it works and deny_ptrace is on, emit a specific warning about it.  If
  it doesn't work or if deny_ptrace is off, continue.

- Try to check the contents of /proc/sys/kernel/yama/ptrace_scope.  If
  it works and if ptrace_scope > 0, emit a specific warning about it.
  Otherwise, continue.

- See if I can come up with a way to check the seccomp thing.  If
  possible, emit a specific warning about it.  Otherwise, continue.

- If everything failed, emit a generic warning pointing to the docs.

I hope this is a better approach.

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] 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: Sergio Durigan Junior <[hidden email]>
> Date: Sun, 18 Aug 2019 23:29:18 -0400
>
> +@node Linux kernel @code{ptrace} restrictions

@-commands in node names are best avoided (here and elsewhere in your
patch).

> +The @code{ptrace} system call is used by @value{GDBN} to, among other
> +things, attach to a new or existing inferior in order to start
> +debugging it.

This sentence should mention Linux, otherwise it's too general.

> +we will expand on how this malfunction can manifest, and how to modify
                                              ^^^^^^^^
"manifest itself"

Also, I see no description of how these problems manifest themselves.
I suggest to show the respective error messages, and also index them,
so that interested readers could find this material quickly.

> +@cindex selinux, deny_ptrace

I suggest to add a @cindex entry for deny_ptrace itself, or maybe
switch the order:

  @cindex deny_ptrace, SELinux

> +If you see anything other than @code{0}, @value{GDBN} can be affected
> +by it.  You can temporarily disable the feature by doing:
> +
> +@smallexample
> +# sysctl kernel.yama.ptrace_scope=0
> +kernel.yama.ptrace_scope = 0
> +@end smallexample

I'm guessing this should be done as root, right?  If so, I think we
should mention that.

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

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

Ruslan Kabatsayev
In reply to this post by Sergio Durigan Junior
On Mon, 19 Aug 2019 at 16:47, Sergio Durigan Junior <[hidden email]> wrote:

>
> On Monday, August 19 2019, Ruslan Kabatsayev wrote:
>
> > Hello Sergio,
>
> Hey Ruslan,
>
> > On Mon, 19 Aug 2019 at 06:29, Sergio Durigan Junior <[hidden email]> wrote:
> >>
> >> In Fedora GDB, we carry the following patch:
> >>
> >>   https://src.fedoraproject.org/rpms/gdb/blob/master/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.
> >>
> >> Instead of printing a very certain warning about a very specific
> >> setting, I decided to propose the following patch, which prints a
> >> generic warning about a possible situation (i.e., without going
> >> bothering to make sure that the situation is actually happening).
> >> What this means is that whenever a ptrace error is detected, and if
> >> errno is either EACCES or EPERM, a warning will be printed pointing
> >> the user to our documentation, where she will find more details about
> >> possible restrictions in place against ptrace.
> >
> > Doesn't it worsen user experience compared to the current Fedora
> > patch? Now a user first coming across the problem will have to
> > navigate the documentation, then check possible scenarios – instead of
> > simply getting a friendly explanation what exactly is wrong.
>
> I agree that it is easier to just print the reason for the failure
> directly, instead of pointing to the documentation.  However, the reason
> I chose the latter is because it may not always be possible to identify
> the reason, and this might confuse the user or lead him to wrong
> conclusions.  I'm mostly thinking about the seccomp feature here
> (because I have no idea how to check if it's active or not), but I'm
> also concerned that it's unreasonable to expect users to have
> libselinux-devel (on Fedora) installed.
>
> > Compare this with the patch applied by some Linux distributions
> > (CentOS IIRC) to make Bash emit
> >
> >> bash: myprogram: /lib/ld-linux.so.2: bad ELF interpreter: No such file or directory
> >
> > when ELF interpreter can't be found. By default, Bash only prints the
> > less than helpful message
> >
> >> bash: myprogram: No such file or directory
> >
> > Your proposal (if it's intended to replace Fedora's GDB patch) is
> > similar to replacing this default with
> >
> >> bash: myprogram: No such file or directory. There might be other reasons than simply command executable not existing. Please see Bash documentation section XYZ for details.
> >
> > and adding a section XYZ in Bash docs explaining that the reasons
> > could include lacking libraries, broken symlink to the binary, stale
> > hash table of PATH lookups, etc.. This is considerably worse than the
> > patch with explicit announcement of actual problem.
>
> Well, in this specific case it's probably not difficult to check for the
> right cause and print the specific warning.  However, if there were more
> than 2 or 3 scenarios where a program couldn't be executed, and if
> checking for them involved depending on external libraries and such,
> then we'd be talking about a similar situation here.
>
> > So as a user, I disfavor this approach, although it might be more
> > maintainable for GDB developers. But, if dependency on libselinux is
> > the only objection, then why not simply dlopen it as needed (and
> > gracefully handle failure to do so) instead of ELF-level linking to
> > it? Then there'll be no hard dependency on libselinux, while user
> > experience will not degrade too.
>
> Thanks for expressing your opinion as user, much appreciated!
>
> I had considered dlopen'ing libselinux before, but didn't do that
> because of what I mentioned earlier: I think it's not reasonable to
> expect that the user will have libselinux-devel installed, and SELinux
> is just one of the possible scenarios here.
>
> But your message made me rethink and decide to do the following:
>
> - Try to dlopen libselinux, and check for deny_ptrace if possible.  If
>   it works and deny_ptrace is on, emit a specific warning about it.  If
>   it doesn't work or if deny_ptrace is off, continue.
>
> - Try to check the contents of /proc/sys/kernel/yama/ptrace_scope.  If
>   it works and if ptrace_scope > 0, emit a specific warning about it.
>   Otherwise, continue.
>
> - See if I can come up with a way to check the seccomp thing.  If
>   possible, emit a specific warning about it.  Otherwise, continue.
>
> - If everything failed, emit a generic warning pointing to the docs.

Yes, I did consider suggesting the same, just thought it would look
too complicated to have both generic and specific cases. I'm glad that
you've come to this solution anyway.

>
> I hope this is a better approach.
>
> 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/

Thanks,
Ruslan
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
In reply to this post by Sergio Durigan Junior
On Mon, Aug 19, 2019 at 8:47 AM Sergio Durigan Junior
<[hidden email]> wrote:
> I'm
> also concerned that it's unreasonable to expect users to have
> libselinux-devel (on Fedora) installed.

I'm confused, why would users have to have the -dev version installed?
Can't you just dlopen("libselinux.so.1") and it'll work fine without
the -dev package?

Christian
Reply | Threaded
Open this post in threaded view
|

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

Sergio Durigan Junior
On Monday, August 19 2019, Christian Biesinger wrote:

> On Mon, Aug 19, 2019 at 8:47 AM Sergio Durigan Junior
> <[hidden email]> wrote:
>> I'm
>> also concerned that it's unreasonable to expect users to have
>> libselinux-devel (on Fedora) installed.
>
> I'm confused, why would users have to have the -dev version installed?
> Can't you just dlopen("libselinux.so.1") and it'll work fine without
> the -dev package?

libselinux.so is present in the libselinux-devel package, but
libselinux.so.1 is present in the libselinux package.  We could dlopen
it, sure.  I don't know if libselinux is installed by default, though.

--
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] Improve ptrace-error detection on Linux targets

Pedro Alves-7
In reply to this post by Sergio Durigan Junior
Two comments on implementation, that I think apply
equally to the new version you're working on as well:

#1 - A target method (target_ptrace_me_fail_reason) looks like the
  wrong place to put this, to me.  As I understand it, the core of
  gdb isn't ever calling the target_ptrace_me_fail_reason.  This is
  all code that is implementation detail of the Linux target
  implementation.

  I'd prefer some other way to address this.  An idea would be to have
  a function pointer in inf-ptrace.c that defaults to an implementation
  that returns the empty string:

   // in inf-ptrace.c

   static std::string
   default_inf_ptrace_me_fail_reason (int err)
   {
     return {};
   }

   std::string (*inf_ptrace_me_fail_reason) (int err) = default_inf_ptrace_me_fail_reason;

   // in inf-ptrace.h

   extern std::string (*inf_ptrace_me_fail_reason) (int err);

   And then in linux-nat.c:_initialize_linux_nat, you'd override the
   implementation, like:

   std::string
   linux_ptrace_me_fail_reason (int err)
   {
     ...
   }

   void
   _initialize_linux_nat ()
   {
      ...
      inf_ptrace_me_fail_reason = linux_ptrace_me_fail_reason;
   }


  Note I'm not suggesting anything with virtual methods before
  of the next point.

#2 - What about gdbserver?  Don't we want the same nice error
messages in gdbserver?

See gdbserver/linux-low.c:linux_ptrace_fun.

This would suggest putting that linux_ptrace_me_fail_reason
function in gdb/nat/ so that it would be used by gdbserver too.

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

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

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

>> From: Sergio Durigan Junior <[hidden email]>
>> Cc: Sergio Durigan Junior <[hidden email]>
>> Date: Sun, 18 Aug 2019 23:29:18 -0400
>>
>> +@node Linux kernel @code{ptrace} restrictions
>
> @-commands in node names are best avoided (here and elsewhere in your
> patch).

Fixed.

>> +The @code{ptrace} system call is used by @value{GDBN} to, among other
>> +things, attach to a new or existing inferior in order to start
>> +debugging it.
>
> This sentence should mention Linux, otherwise it's too general.

I thought mentioning Linux in the title would be enough.  I've modified
the sentence to read like this:

  The @code{ptrace} system call is used by @value{GDBN} on Linux to,
  among other things...

>> +we will expand on how this malfunction can manifest, and how to modify
>                                               ^^^^^^^^
> "manifest itself"

Fixed.

> Also, I see no description of how these problems manifest themselves.
> I suggest to show the respective error messages, and also index them,
> so that interested readers could find this material quickly.

As per our recent discussions in the thread, I changed the patch to
actively try to determine the cause of the ptrace failure, and print
descriptive messages for each scenario.  When it isn't able to detect
the root cause, it will print the generic message pointing the user to
our documentation.  Given this change in the original behaviour, I'm
probably going to rewrite this sentence so that it's more clear about
what should be expected.

>> +@cindex selinux, deny_ptrace
>
> I suggest to add a @cindex entry for deny_ptrace itself, or maybe
> switch the order:
>
>   @cindex deny_ptrace, SELinux

Fair enough, I'll add a separate entry for deny_ptrace.

>> +If you see anything other than @code{0}, @value{GDBN} can be affected
>> +by it.  You can temporarily disable the feature by doing:
>> +
>> +@smallexample
>> +# sysctl kernel.yama.ptrace_scope=0
>> +kernel.yama.ptrace_scope = 0
>> +@end smallexample
>
> I'm guessing this should be done as root, right?  If so, I think we
> should mention that.

Yes, this command should be run as root.  I thought that just using "#"
vs. "$" in the prompt would suffice, but you're right, I should be more
explicit.

I'll send a v2 with the fixes soon.

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] Improve ptrace-error detection on Linux targets

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

> Two comments on implementation, that I think apply
> equally to the new version you're working on as well:

Thanks for the review.  I didn't reply before, but I used your
suggestions in the next version.

> #1 - A target method (target_ptrace_me_fail_reason) looks like the
>   wrong place to put this, to me.  As I understand it, the core of
>   gdb isn't ever calling the target_ptrace_me_fail_reason.  This is
>   all code that is implementation detail of the Linux target
>   implementation.
>
>   I'd prefer some other way to address this.  An idea would be to have
>   a function pointer in inf-ptrace.c that defaults to an implementation
>   that returns the empty string:
>
>    // in inf-ptrace.c
>
>    static std::string
>    default_inf_ptrace_me_fail_reason (int err)
>    {
>      return {};
>    }
>
>    std::string (*inf_ptrace_me_fail_reason) (int err) = default_inf_ptrace_me_fail_reason;
>
>    // in inf-ptrace.h
>
>    extern std::string (*inf_ptrace_me_fail_reason) (int err);
>
>    And then in linux-nat.c:_initialize_linux_nat, you'd override the
>    implementation, like:
>
>    std::string
>    linux_ptrace_me_fail_reason (int err)
>    {
>      ...
>    }
>
>    void
>    _initialize_linux_nat ()
>    {
>       ...
>       inf_ptrace_me_fail_reason = linux_ptrace_me_fail_reason;
>    }
>
>
>   Note I'm not suggesting anything with virtual methods before
>   of the next point.
>
> #2 - What about gdbserver?  Don't we want the same nice error
> messages in gdbserver?
>
> See gdbserver/linux-low.c:linux_ptrace_fun.
>
> This would suggest putting that linux_ptrace_me_fail_reason
> function in gdb/nat/ so that it would be used by gdbserver too.

Thanks.  I followed both of your suggestions, and will submit a v2 soon.

--
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 v2] Improve ptrace-error detection on Linux targets

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

- Addressed Pedro's comments re. internal organization and gdbserver
  support.

- Addressed Eli's comments (doc fixes).

- New ways of detecting what's wrong with ptrace.



In Fedora GDB, we carry the following patch:

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

Its purpose is to try to detect a specific scenario where SELinux's
'deny_ptrace' option is enabled, which prevents GDB from ptrace'ing in
order to debug the inferior (PTRACE_ATTACH and PTRACE_ME will fail
with EACCES in this case).

I like the idea of improving error detection and providing more
information to the user (a simple "Permission denied" can be really
frustrating), but I don't fully agree with the way the patch was
implemented: it makes GDB link against libselinux only for the sake of
consulting the 'deny_ptrace' setting, and then prints a warning if
ptrace failed and this setting is on.

My first thought (and attempt) was to make GDB print a generic warning
when a ptrace error happened; this message would just point the user
to our documentation, where she could find more information about
possible causes for the error (and try to diagnose/fix the problem).
This proved to be too simple, and I was convinced that it is actually
a good idea to go the extra kilometre and try to pinpoint the specific
problem (or problems) preventing ptrace from working, as well as
provide useful suggestions on how the user can fix things.

Here is the patch I came up with.  It implements a new function,
'linux_ptrace_restricted_fail_reason', which does a few things to
check what's wrong with ptrace:

  - It dlopen's "libselinux.so" and checks if the "deny_ptrace" option
    is enabled.

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

    setsebool deny_ptrace off

  During startup program exited with code 127.

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

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

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

  During startup program exited with code 127.

If both restrictions are enabled, both messages will show up.

This works for gdbserver as well, and actually fixes a latent bug I
found: when ptrace is restricted, gdbserver would hang due to an
unchecked ptrace call:

  # gdbserver :9988 /bin/true
  gdbserver: linux_ptrace_test_ret_to_nx: Cannot PTRACE_TRACEME: Operation not permitted
  gdbserver: linux_ptrace_test_ret_to_nx: status 256 is not WIFSTOPPED!
  gdbserver: linux_ptrace_test_ret_to_nx: failed to kill child pid 2668100 No such process
  [ Here you would have to issue a C-c ]

Now, you will see:

  # gdbserver :9988 /bin/true
  gdbserver: linux_ptrace_test_ret_to_nx: Cannot PTRACE_TRACEME: Operation not permitted
  gdbserver: linux_ptrace_test_ret_to_nx: status 256 is not WIFSTOPPED!
  gdbserver: linux_ptrace_test_ret_to_nx: failed to kill child pid 2668100 No such process
  gdbserver: Could not trace the inferior process.
  Error:
  gdbserver: ptrace: Operation not permitted
  The Linux kernel's Yama ptrace scope is in effect, which can prevent
  GDB from using 'ptrace'.  Please, disable it by executing (as root):

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

  linux_check_ptrace_features: waitpid: unexpected status 32512.
  Exiting

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

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]>

        * gdbsupport/gdb-dlfcn.c (gdb_dlopen): Add argument 'dont_throw'
        Don't throw if it's true.
        * gdbsupport/gdb-dlfcn.h (gdb_dlopen): Add optional argument
        'dont_throw'.  Update comment.
        * inf-ptrace.c (default_inf_ptrace_me_fail_reason): New
        function.
        (inf_ptrace_me_fail_reason): New variable.
        (inf_ptrace_me): Update call to 'trace_start_error_with_name'.
        * inf-ptrace.h (inf_ptrace_me_fail_reason): New variable.
        * linux-nat.c (linux_nat_target::attach): Update call to
        'linux_ptrace_attach_fail_reason'.
        (_initialize_linux_nat): Set 'inf_ptrace_me_fail_reason'.
        * linux-nat.h (linux_nat_target) <ptrace_me_fail_reason>: New
        method.
        * nat/fork-inferior.c (trace_start_error_with_name): Add
        optional 'append' argument.
        * nat/fork-inferior.h (trace_start_error_with_name): Update
        prototype.
        * nat/linux-ptrace.c: Include "gdbsupport/gdb-dlfcn.h" and
        "nat/fork-inferior.h".
        (selinux_ftype): New typedef.
        (linux_ptrace_restricted_fail_reason): New function.
        (linux_ptrace_attach_fail_reason): Add optional 'err'
        argument.  Call 'linux_ptrace_restricted_fail_reason'.
        (linux_ptrace_me_fail_reason): New function.
        (linux_child_function): Handle ptrace error.
        * nat/linux-ptrace.h (linux_ptrace_attach_fail_reason): Update
        prototype.
        (linux_ptrace_me_fail_reason): New function.
        * target-delegates.c: Regenerate.
        * target.h (struct target_ops) <ptrace_me_fail_reason>: New
        method.
        (target_ptrace_me_fail_reason): New define.

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

        * linux-low.c (linux_ptrace_fun): Call
        'linux_ptrace_me_fail_reason'.
---
 gdb/doc/gdb.texinfo        | 138 +++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/linux-low.c  |   3 +-
 gdb/gdbsupport/gdb-dlfcn.c |   4 +-
 gdb/gdbsupport/gdb-dlfcn.h |   7 +-
 gdb/inf-ptrace.c           |  19 ++++-
 gdb/inf-ptrace.h           |  10 +++
 gdb/linux-nat.c            |   7 +-
 gdb/nat/fork-inferior.c    |   4 +-
 gdb/nat/fork-inferior.h    |   7 +-
 gdb/nat/linux-ptrace.c     |  86 +++++++++++++++++++++--
 gdb/nat/linux-ptrace.h     |  15 +++-
 11 files changed, 282 insertions(+), 18 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e1bc8143e6..43f749c6f1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -182,6 +182,9 @@ software in general.  We will miss him.
                                 @value{GDBN}
 * Operating System Information:: Getting additional information from
                                  the operating system
+* Linux kernel @code{ptrace} restrictions:: Restrictions sometimes
+                                            imposed by the Linux
+                                            kernel on @code{ptrace}
 * Trace File Format:: GDB trace file format
 * Index Section Format::        .gdb_index section format
 * Man Pages:: Manual pages
@@ -44682,6 +44685,141 @@ should contain a comma-separated list of cores that this process
 is running on.  Target may provide additional columns,
 which @value{GDBN} currently ignores.
 
+@node Linux kernel ptrace restrictions
+@appendix Linux kernel @code{ptrace} restrictions
+@cindex linux kernel ptrace restrictions, attach
+
+The @code{ptrace} system call is used by @value{GDBN} on GNU/Linux to,
+among other things, attach to a new or existing inferior in order to
+start debugging it.  Due to security concerns, some distributions and
+vendors disable or severily restrict the ability to perform these
+operations, which can make @value{GDBN} malfunction.  In this section,
+we will expand on how this malfunction can manifest itself, and how to
+modify the system's settings in order to be able to use @value{GDBN}
+properly.
+
+@menu
+* The GDB error message::               The error message displayed when the
+                                        system prevents @value{GDBN} from using
+                                        @code{ptrace}
+* SELinux's @code{deny_ptrace}::        SELinux and the @code{deny_ptrace} option
+* Yama's @code{ptrace_scope}::          Yama and the @code{ptrace_scope} setting
+* Docker and @code{seccomp}::           Docker and the @code{seccomp}
+                                        infrastructure
+@end menu
+
+@node The GDB error message
+@appendixsection The @value{GDBN} error message
+
+When the system prevents @value{GDBN} from using the @code{ptrace}
+system call, you will likely see a descriptive error message
+explaining what is wrong and how to attempt to fix the problem.  For
+example, when SELinux's @code{deny_ptrace} option is enabled, you can
+see:
+
+@smallexample
+$ gdb program
+...
+(@value{GDBP}) run
+Starting program: program
+warning: Could not trace the inferior process.
+Error:
+warning: ptrace: Permission denied
+The SELinux 'deny_ptrace' option is enabled and preventing GDB
+from using 'ptrace'.  Please, disable it by executing (as root):
+
+  setsebool deny_ptrace off
+
+During startup program exited with code 127.
+(@value{GDBP})
+@end smallexample
+
+Sometimes, it may not be possible to acquire the necessary data to
+determine the root cause of the failure.  In this case, you will see a
+generic error message pointing you to this section:
+
+@smallexample
+$ gdb program
+...
+Starting program: program
+warning: Could not trace the inferior process.
+Error:
+warning: ptrace: Permission denied
+There might be restrictions preventing ptrace from working.  Please see
+the appendix "Linux kernel ptrace restrictions" in the GDB documentation
+for more details.
+During startup program exited with code 127.
+(@value{GDBP})
+@end smallexample
+
+@node SELinux's deny_ptrace
+@appendixsection SELinux's @code{deny_ptrace}
+@cindex SELinux
+@cindex deny_ptrace
+
+If you are using SELinux, you might want to check whether the
+@code{deny_ptrace} option is enabled by doing:
+
+@smallexample
+$ getsebool deny_ptrace
+deny_ptrace --> on
+@end smallexample
+
+If the option is enabled, you can disable it by doing, as root:
+
+@smallexample
+# setsebool deny_ptrace off
+@end smallexample
+
+The option will be disabled until the next reboot.  If you would like
+to disable it permanently, you can do (as root):
+
+@smallexample
+# setsebool -P deny_ptrace off
+@end smallexample
+
+@node Yama's ptrace_scope
+@appendixsection Yama's @code{ptrace_scope}
+@cindex yama, ptrace_scope
+
+If your system has Yama enabled, you might want to check whether the
+@code{ptrace_scope} setting is enabled by checking the value of
+@file{/proc/sys/kernel/yama/ptrace_scope}:
+
+@smallexample
+$ cat /proc/sys/kernel/yama/ptrace_scope
+0
+@end smallexample
+
+If you see anything other than @code{0}, @value{GDBN} can be affected
+by it.  You can temporarily disable the feature by doing, as root:
+
+@smallexample
+# sysctl kernel.yama.ptrace_scope=0
+kernel.yama.ptrace_scope = 0
+@end smallexample
+
+You can make this permanent by doing, as root:
+
+@smallexample
+# sysctl -w kernel.yama.ptrace_scope=0
+kernel.yama.ptrace_scope = 0
+@end smallexample
+
+@node Docker and seccomp
+@appendixsection Docker and @code{seccomp}
+@cindex docker, seccomp
+
+If you are using Docker (@uref{https://www.docker.com/}) containers,
+you will probably have to disable its @code{seccomp} protections in
+order to be able to use @value{GDBN}.  To do that, you can use the
+options @code{--cap-add=SYS_PTRACE --security-opt seccomp=unconfined}
+when invoking Docker:
+
+@smallexample
+$ docker run --cap-add=SYS_PTRACE --security-opt seccomp=unconfined
+@end smallexample
+
 @node Trace File Format
 @appendix Trace File Format
 @cindex trace file format
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 3113017ae6..1e0a5cbf54 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -971,7 +971,8 @@ linux_ptrace_fun ()
 {
   if (ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0,
       (PTRACE_TYPE_ARG4) 0) < 0)
-    trace_start_error_with_name ("ptrace");
+    trace_start_error_with_name ("ptrace",
+ linux_ptrace_me_fail_reason (errno).c_str ());
 
   if (setpgid (0, 0) < 0)
     trace_start_error_with_name ("setpgid");
diff --git a/gdb/gdbsupport/gdb-dlfcn.c b/gdb/gdbsupport/gdb-dlfcn.c
index 921f10f3d8..9e5a992c17 100644
--- a/gdb/gdbsupport/gdb-dlfcn.c
+++ b/gdb/gdbsupport/gdb-dlfcn.c
@@ -58,7 +58,7 @@ is_dl_available (void)
 #else /* NO_SHARED_LIB */
 
 gdb_dlhandle_up
-gdb_dlopen (const char *filename)
+gdb_dlopen (const char *filename, bool dont_throw)
 {
   void *result;
 #ifdef HAVE_DLFCN_H
@@ -66,7 +66,7 @@ gdb_dlopen (const char *filename)
 #elif __MINGW32__
   result = (void *) LoadLibrary (filename);
 #endif
-  if (result != NULL)
+  if (dont_throw || result != NULL)
     return gdb_dlhandle_up (result);
 
 #ifdef HAVE_DLFCN_H
diff --git a/gdb/gdbsupport/gdb-dlfcn.h b/gdb/gdbsupport/gdb-dlfcn.h
index 6a39d38941..a8ddbc03da 100644
--- a/gdb/gdbsupport/gdb-dlfcn.h
+++ b/gdb/gdbsupport/gdb-dlfcn.h
@@ -32,10 +32,11 @@ struct dlclose_deleter
 typedef std::unique_ptr<void, dlclose_deleter> gdb_dlhandle_up;
 
 /* Load the dynamic library file named FILENAME, and return a handle
-   for that dynamic library.  Return NULL if the loading fails for any
-   reason.  */
+   for that dynamic library.  If the loading fails, return NULL if
+   DONT_THROW is true, or throw an exception otherwise (default
+   behaviour).  */
 
-gdb_dlhandle_up gdb_dlopen (const char *filename);
+gdb_dlhandle_up gdb_dlopen (const char *filename, bool dont_throw = false);
 
 /* Return the address of the symbol named SYMBOL inside the shared
    library whose handle is HANDLE.  Return NULL when the symbol could
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 4a8e732373..b6cfd803cf 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -94,6 +94,22 @@ inf_ptrace_target::remove_fork_catchpoint (int pid)
 #endif /* PT_GET_PROCESS_STATE */
 
 
+/* Default method for "inf_ptrace_me_fail_reason", which returns an
+   empty string.  */
+
+static std::string
+default_inf_ptrace_me_fail_reason (int err)
+{
+  return {};
+}
+
+/* Point to "inf_ptrace_me_fail_reason", which implements a function
+   that can be called by "inf_ptrace_me" in order to obtain the reason
+   for failure.  */
+
+std::string (*inf_ptrace_me_fail_reason) (int err)
+  = default_inf_ptrace_me_fail_reason;
+
 /* Prepare to be traced.  */
 
 static void
@@ -101,7 +117,8 @@ inf_ptrace_me (void)
 {
   /* "Trace me, Dr. Memory!"  */
   if (ptrace (PT_TRACE_ME, 0, (PTRACE_TYPE_ARG3) 0, 0) < 0)
-    trace_start_error_with_name ("ptrace");
+    trace_start_error_with_name ("ptrace",
+ inf_ptrace_me_fail_reason (errno).c_str ());
 }
 
 /* Start a new inferior Unix child process.  EXEC_FILE is the file to
diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h
index 98b5d2e09e..7cdab9af89 100644
--- a/gdb/inf-ptrace.h
+++ b/gdb/inf-ptrace.h
@@ -83,4 +83,14 @@ protected:
 
 extern pid_t get_ptrace_pid (ptid_t);
 
+/* Pointer to "inf_ptrace_me_fail_reason", which implements a function
+   that can be called by "inf_ptrace_me" in order to obtain the reason
+   for a ptrace failure.  ERR is the ERRNO value set by the failing
+   ptrace call.
+
+   This pointer can be overriden by targets that want to personalize
+   the error message printed when ptrace fails (see linux-nat.c, for
+   example).  */
+extern std::string (*inf_ptrace_me_fail_reason) (int err);
+
 #endif
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 945c19f666..b5a9eaf72e 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1191,8 +1191,9 @@ linux_nat_target::attach (const char *args, int from_tty)
     }
   catch (const gdb_exception_error &ex)
     {
+      int saved_errno = errno;
       pid_t pid = parse_pid_to_attach (args);
-      std::string reason = linux_ptrace_attach_fail_reason (pid);
+      std::string reason = linux_ptrace_attach_fail_reason (pid, saved_errno);
 
       if (!reason.empty ())
  throw_error (ex.error, "warning: %s\n%s", reason.c_str (),
@@ -4696,6 +4697,10 @@ Enables printf debugging output."),
   sigemptyset (&blocked_mask);
 
   lwp_lwpid_htab_create ();
+
+  /* Set the proper function to generate a message when ptrace
+     fails.  */
+  inf_ptrace_me_fail_reason = linux_ptrace_me_fail_reason;
 }
 
 
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 68b51aa814..72ac623e20 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -591,7 +591,7 @@ trace_start_error (const char *fmt, ...)
 /* See nat/fork-inferior.h.  */
 
 void
-trace_start_error_with_name (const char *string)
+trace_start_error_with_name (const char *string, const char *append)
 {
-  trace_start_error ("%s: %s", string, safe_strerror (errno));
+  trace_start_error ("%s: %s%s", string, safe_strerror (errno), append);
 }
diff --git a/gdb/nat/fork-inferior.h b/gdb/nat/fork-inferior.h
index 1d0519fb26..7e6b889210 100644
--- a/gdb/nat/fork-inferior.h
+++ b/gdb/nat/fork-inferior.h
@@ -98,9 +98,10 @@ extern void trace_start_error (const char *fmt, ...)
   ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
 
 /* Like "trace_start_error", but the error message is constructed by
-   combining STRING with the system error message for errno.  This
-   function does not return.  */
-extern void trace_start_error_with_name (const char *string)
+   combining STRING with the system error message for errno, and
+   (optionally) with APPEND.  This function does not return.  */
+extern void trace_start_error_with_name (const char *string,
+ const char *append = "")
   ATTRIBUTE_NORETURN;
 
 #endif /* NAT_FORK_INFERIOR_H */
diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
index c1ebc0a032..599d9cfb55 100644
--- a/gdb/nat/linux-ptrace.c
+++ b/gdb/nat/linux-ptrace.c
@@ -21,6 +21,8 @@
 #include "linux-procfs.h"
 #include "linux-waitpid.h"
 #include "gdbsupport/buffer.h"
+#include "gdbsupport/gdb-dlfcn.h"
+#include "nat/fork-inferior.h"
 #ifdef HAVE_SYS_PROCFS_H
 #include <sys/procfs.h>
 #endif
@@ -30,11 +32,70 @@
    of 0 means there are no supported features.  */
 static int supported_ptrace_options = -1;
 
-/* Find all possible reasons we could fail to attach PID and return these
-   as a string.  An empty string is returned if we didn't find any reason.  */
+typedef int (*selinux_ftype) (const char *);
+
+/* Helper function which checks if ptrace is probably restricted
+   (i.e., if ERR is either EACCES or EPERM), and returns a string with
+   possible workarounds.  */
+
+static std::string
+linux_ptrace_restricted_fail_reason (int err)
+{
+  if (err != EACCES && err != EPERM)
+    {
+      /* It just makes sense to perform the checks below if errno was
+ either EACCES or EPERM.  */
+      return {};
+    }
+
+  std::string ret;
+  gdb_dlhandle_up handle = gdb_dlopen ("libselinux.so", true);
+
+  if (handle.get () != NULL)
+    {
+      selinux_ftype selinux_get_bool
+ = (selinux_ftype) gdb_dlsym (handle, "security_get_boolean_active");
+
+      if (selinux_get_bool != NULL
+  && (*selinux_get_bool) ("deny_ptrace") == 1)
+ string_appendf (ret,
+ _("\n\
+The SELinux 'deny_ptrace' option is enabled and preventing GDB\n\
+from using 'ptrace'.  Please, disable it by executing (as root):\n\
+\n\
+  setsebool deny_ptrace off\n"));
+    }
+
+  FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r");
+
+  if (f != NULL)
+    {
+      char yama_scope = fgetc (f);
+
+      fclose (f);
+
+      if (yama_scope != '0')
+ string_appendf (ret,
+ _("\n\
+The Linux kernel's Yama ptrace scope is in effect, which can prevent\n\
+GDB from using 'ptrace'.  Please, disable it by executing (as root):\n\
+\n\
+  echo 0 > /proc/sys/kernel/yama/ptrace_scope\n"));
+    }
+
+  if (ret.empty ())
+    ret = _("\n\
+There might be restrictions preventing ptrace from working.  Please see\n\
+the appendix \"Linux kernel ptrace restrictions\" in the GDB documentation\n\
+for more details.\n");
+
+  return ret;
+}
+
+/* See declaration in linux-ptrace.h.  */
 
 std::string
-linux_ptrace_attach_fail_reason (pid_t pid)
+linux_ptrace_attach_fail_reason (pid_t pid, int err)
 {
   pid_t tracerpid = linux_proc_get_tracerpid_nowarn (pid);
   std::string result;
@@ -50,6 +111,11 @@ linux_ptrace_attach_fail_reason (pid_t pid)
       "terminated"),
     (int) pid);
 
+  std::string ptrace_restrict = linux_ptrace_restricted_fail_reason (err);
+
+  if (!ptrace_restrict.empty ())
+    result += "\n" + ptrace_restrict;
+
   return result;
 }
 
@@ -68,6 +134,14 @@ linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err)
     return string_printf ("%s (%d)", safe_strerror (err), err);
 }
 
+/* See linux-ptrace.h.  */
+
+std::string
+linux_ptrace_me_fail_reason (int err)
+{
+  return linux_ptrace_restricted_fail_reason (err);
+}
+
 #if defined __i386__ || defined __x86_64__
 
 /* Address of the 'ret' instruction in asm code block below.  */
@@ -321,7 +395,11 @@ linux_grandchild_function (void *child_stack)
 static int
 linux_child_function (void *child_stack)
 {
-  ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) 0);
+  if (ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0,
+      (PTRACE_TYPE_ARG4) 0) != 0)
+    trace_start_error_with_name ("ptrace",
+ linux_ptrace_me_fail_reason (errno).c_str ());
+
   kill (getpid (), SIGSTOP);
 
   /* Fork a grandchild.  */
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index fd2f12a342..04ada53bf6 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -176,13 +176,26 @@ struct buffer;
 # define TRAP_HWBKPT 4
 #endif
 
-extern std::string linux_ptrace_attach_fail_reason (pid_t pid);
+/* Find all possible reasons we could fail to attach PID and return
+   these as a string.  An empty string is returned if we didn't find
+   any reason.  If ERR is EACCES or EPERM, we also add a warning about
+   possible restrictions to use ptrace.  */
+extern std::string linux_ptrace_attach_fail_reason (pid_t pid, int err = -1);
 
 /* Find all possible reasons we could have failed to attach to PTID
    and return them as a string.  ERR is the error PTRACE_ATTACH failed
    with (an errno).  */
 extern std::string linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err);
 
+/* When the call to 'ptrace (PTRACE_TRACEME...' fails, and we have
+   already forked, this function can be called in order to try to
+   obtain the reason why ptrace failed.  ERR should be the ERRNO value
+   returned by ptrace.
+
+   This function will return a 'std::string' containing the fail
+   reason, or an empty string otherwise.  */
+extern std::string linux_ptrace_me_fail_reason (int err);
+
 extern void linux_ptrace_init_warnings (void);
 extern void linux_check_ptrace_features (void);
 extern void linux_enable_event_reporting (pid_t pid, int attached);
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
On Mon, Aug 26, 2019 at 1:32 PM Sergio Durigan Junior
<[hidden email]> wrote:

> @@ -30,11 +32,70 @@
>     of 0 means there are no supported features.  */
>  static int supported_ptrace_options = -1;
>
> -/* Find all possible reasons we could fail to attach PID and return these
> -   as a string.  An empty string is returned if we didn't find any reason.  */
> +typedef int (*selinux_ftype) (const char *);
> +
> +/* Helper function which checks if ptrace is probably restricted
> +   (i.e., if ERR is either EACCES or EPERM), and returns a string with
> +   possible workarounds.  */
> +
> +static std::string
> +linux_ptrace_restricted_fail_reason (int err)
> +{
> +  if (err != EACCES && err != EPERM)
> +    {
> +      /* It just makes sense to perform the checks below if errno was
> +        either EACCES or EPERM.  */
> +      return {};
> +    }
> +
> +  std::string ret;
> +  gdb_dlhandle_up handle = gdb_dlopen ("libselinux.so", true);

I would dlopen libselinux.so.1 instead so that this works even without
the -dev package.

> +
> +  if (handle.get () != NULL)
> +    {
> +      selinux_ftype selinux_get_bool
> +       = (selinux_ftype) gdb_dlsym (handle, "security_get_boolean_active");
> +
> +      if (selinux_get_bool != NULL
> +         && (*selinux_get_bool) ("deny_ptrace") == 1)
> +       string_appendf (ret,
> +                       _("\n\
> +The SELinux 'deny_ptrace' option is enabled and preventing GDB\n\
> +from using 'ptrace'.  Please, disable it by executing (as root):\n\
> +\n\
> +  setsebool deny_ptrace off\n"));
> +    }
> +
> +  FILE *f = fopen ("/proc/sys/kernel/yama/ptrace_scope", "r");
> +
> +  if (f != NULL)
> +    {
> +      char yama_scope = fgetc (f);
> +
> +      fclose (f);
> +
> +      if (yama_scope != '0')
> +       string_appendf (ret,
> +                       _("\n\
> +The Linux kernel's Yama ptrace scope is in effect, which can prevent\n\
> +GDB from using 'ptrace'.  Please, disable it by executing (as root):\n\
> +\n\
> +  echo 0 > /proc/sys/kernel/yama/ptrace_scope\n"));
> +    }
> +
> +  if (ret.empty ())
> +    ret = _("\n\
> +There might be restrictions preventing ptrace from working.  Please see\n\
> +the appendix \"Linux kernel ptrace restrictions\" in the GDB documentation\n\
> +for more details.\n");
> +
> +  return ret;
> +}
> +
> +/* See declaration in linux-ptrace.h.  */
>
>  std::string
> -linux_ptrace_attach_fail_reason (pid_t pid)
> +linux_ptrace_attach_fail_reason (pid_t pid, int err)
>  {
>    pid_t tracerpid = linux_proc_get_tracerpid_nowarn (pid);
>    std::string result;
> @@ -50,6 +111,11 @@ linux_ptrace_attach_fail_reason (pid_t pid)
>                       "terminated"),
>                     (int) pid);
>
> +  std::string ptrace_restrict = linux_ptrace_restricted_fail_reason (err);
> +
> +  if (!ptrace_restrict.empty ())
> +    result += "\n" + ptrace_restrict;
> +
>    return result;
>  }
>
> @@ -68,6 +134,14 @@ linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err)
>      return string_printf ("%s (%d)", safe_strerror (err), err);
>  }
>
> +/* See linux-ptrace.h.  */
> +
> +std::string
> +linux_ptrace_me_fail_reason (int err)
> +{
> +  return linux_ptrace_restricted_fail_reason (err);
> +}
> +
>  #if defined __i386__ || defined __x86_64__
>
>  /* Address of the 'ret' instruction in asm code block below.  */
> @@ -321,7 +395,11 @@ linux_grandchild_function (void *child_stack)
>  static int
>  linux_child_function (void *child_stack)
>  {
> -  ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) 0);
> +  if (ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0,
> +             (PTRACE_TYPE_ARG4) 0) != 0)
> +    trace_start_error_with_name ("ptrace",
> +                                linux_ptrace_me_fail_reason (errno).c_str ());
> +
>    kill (getpid (), SIGSTOP);
>
>    /* Fork a grandchild.  */
> diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
> index fd2f12a342..04ada53bf6 100644
> --- a/gdb/nat/linux-ptrace.h
> +++ b/gdb/nat/linux-ptrace.h
> @@ -176,13 +176,26 @@ struct buffer;
>  # define TRAP_HWBKPT 4
>  #endif
>
> -extern std::string linux_ptrace_attach_fail_reason (pid_t pid);
> +/* Find all possible reasons we could fail to attach PID and return
> +   these as a string.  An empty string is returned if we didn't find
> +   any reason.  If ERR is EACCES or EPERM, we also add a warning about
> +   possible restrictions to use ptrace.  */
> +extern std::string linux_ptrace_attach_fail_reason (pid_t pid, int err = -1);
>
>  /* Find all possible reasons we could have failed to attach to PTID
>     and return them as a string.  ERR is the error PTRACE_ATTACH failed
>     with (an errno).  */
>  extern std::string linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err);
>
> +/* When the call to 'ptrace (PTRACE_TRACEME...' fails, and we have
> +   already forked, this function can be called in order to try to
> +   obtain the reason why ptrace failed.  ERR should be the ERRNO value
> +   returned by ptrace.
> +
> +   This function will return a 'std::string' containing the fail
> +   reason, or an empty string otherwise.  */
> +extern std::string linux_ptrace_me_fail_reason (int err);
> +
>  extern void linux_ptrace_init_warnings (void);
>  extern void linux_check_ptrace_features (void);
>  extern void linux_enable_event_reporting (pid_t pid, int attached);
> --
> 2.21.0
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] 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: Mon, 26 Aug 2019 14:32:05 -0400
>
> gdb/doc/ChangeLog:
> yyyy-mm-dd  Sergio Durigan Junior  <[hidden email]>
>
> * gdb.texinfo (Linux kernel ptrace restrictions): New appendix
> section.

OK for this 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 Sourceware - gdb-patches mailing list
On Monday, August 26 2019, Christian Biesinger wrote:

> On Mon, Aug 26, 2019 at 1:32 PM Sergio Durigan Junior
> <[hidden email]> wrote:
>> @@ -30,11 +32,70 @@
>>     of 0 means there are no supported features.  */
>>  static int supported_ptrace_options = -1;
>>
>> -/* Find all possible reasons we could fail to attach PID and return these
>> -   as a string.  An empty string is returned if we didn't find any reason.  */
>> +typedef int (*selinux_ftype) (const char *);
>> +
>> +/* Helper function which checks if ptrace is probably restricted
>> +   (i.e., if ERR is either EACCES or EPERM), and returns a string with
>> +   possible workarounds.  */
>> +
>> +static std::string
>> +linux_ptrace_restricted_fail_reason (int err)
>> +{
>> +  if (err != EACCES && err != EPERM)
>> +    {
>> +      /* It just makes sense to perform the checks below if errno was
>> +        either EACCES or EPERM.  */
>> +      return {};
>> +    }
>> +
>> +  std::string ret;
>> +  gdb_dlhandle_up handle = gdb_dlopen ("libselinux.so", true);
>
> I would dlopen libselinux.so.1 instead so that this works even without
> the -dev package.

Ah, right, I forgot about this.  I fixed it in my local copy, 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
|

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

Pedro Alves-7
In reply to this post by Sergio Durigan Junior
Hi Sergio,

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

On 8/26/19 7:32 PM, Sergio Durigan Junior wrote:

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

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

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

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

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

Curious, that:

 warning:
 Error:
 warning:

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

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

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

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

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

Ditto.

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

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

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

typo: severily -> severely


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

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

which implements a function

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

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

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

No need for .get() here.  This:

  if (handle != NULL)

works the same.

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

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

Use gdb_fopen_cloexec ?

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

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

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

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

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

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

Sergio Durigan Junior
On Thursday, August 29 2019, Pedro Alves wrote:

> Hi Sergio,
>
> This is looking quite good to me.  Some comments below.

Thanks for the review, Pedro.

> On 8/26/19 7:32 PM, Sergio Durigan Junior wrote:
>> Changes from v1:
>>
>> - Addressed Pedro's comments re. internal organization and gdbserver
>>   support.
>>
>> - Addressed Eli's comments (doc fixes).
>>
>> - New ways of detecting what's wrong with ptrace.
>>
>>
>>
>> In Fedora GDB, we carry the following patch:
>>
>>   https://src.fedoraproject.org/rpms/gdb/blob/master/f/gdb-attach-fail-reasons-5of5.patch
>
> This link will soon be dead, right?  (thinking about the commit log)
> Maybe point at some reference other than master.

Yeah.  I will provide a link to a commit.

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

Done.

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

Not offhand; I'll investigate and come back with the results.

>>   The SELinux 'deny_ptrace' option is enabled and preventing GDB
>>   from using 'ptrace'.  Please, disable it by executing (as root):
>
> I'm really not a fan of these "Please, ".  Kind of sounds like
> gdb is begging, to me.  I'd rather use an informational tone, like:
>
>    The SELinux 'deny_ptrace' option is enabled and preventing GDB
>    from using 'ptrace'.  You can disable it by executing (as root):

Fair enough.  Changed as requested.

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

Changed.

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

Agreed.  I can propose a patch later to clean them.

>>
>> 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]>
>>
>> * gdbsupport/gdb-dlfcn.c (gdb_dlopen): Add argument 'dont_throw'
>> Don't throw if it's true.
>> * gdbsupport/gdb-dlfcn.h (gdb_dlopen): Add optional argument
>> 'dont_throw'.  Update comment.
>> * inf-ptrace.c (default_inf_ptrace_me_fail_reason): New
>> function.
>> (inf_ptrace_me_fail_reason): New variable.
>> (inf_ptrace_me): Update call to 'trace_start_error_with_name'.
>> * inf-ptrace.h (inf_ptrace_me_fail_reason): New variable.
>> * linux-nat.c (linux_nat_target::attach): Update call to
>> 'linux_ptrace_attach_fail_reason'.
>> (_initialize_linux_nat): Set 'inf_ptrace_me_fail_reason'.
>> * linux-nat.h (linux_nat_target) <ptrace_me_fail_reason>: New
>> method.
>> * nat/fork-inferior.c (trace_start_error_with_name): Add
>> optional 'append' argument.
>> * nat/fork-inferior.h (trace_start_error_with_name): Update
>> prototype.
>> * nat/linux-ptrace.c: Include "gdbsupport/gdb-dlfcn.h" and
>> "nat/fork-inferior.h".
>> (selinux_ftype): New typedef.
>> (linux_ptrace_restricted_fail_reason): New function.
>> (linux_ptrace_attach_fail_reason): Add optional 'err'
>> argument.  Call 'linux_ptrace_restricted_fail_reason'.
>> (linux_ptrace_me_fail_reason): New function.
>> (linux_child_function): Handle ptrace error.
>> * nat/linux-ptrace.h (linux_ptrace_attach_fail_reason): Update
>> prototype.
>> (linux_ptrace_me_fail_reason): New function.
>> * target-delegates.c: Regenerate.
>> * target.h (struct target_ops) <ptrace_me_fail_reason>: New
>> method.
>> (target_ptrace_me_fail_reason): New define.
>>
>> gdb/gdbserver/ChangeLog:
>> yyyy-mm-dd  Sergio Durigan Junior  <[hidden email]>
>>
>> * linux-low.c (linux_ptrace_fun): Call
>> 'linux_ptrace_me_fail_reason'.
>> ---
>>  gdb/doc/gdb.texinfo        | 138 +++++++++++++++++++++++++++++++++++++
>>  gdb/gdbserver/linux-low.c  |   3 +-
>>  gdb/gdbsupport/gdb-dlfcn.c |   4 +-
>>  gdb/gdbsupport/gdb-dlfcn.h |   7 +-
>>  gdb/inf-ptrace.c           |  19 ++++-
>>  gdb/inf-ptrace.h           |  10 +++
>>  gdb/linux-nat.c            |   7 +-
>>  gdb/nat/fork-inferior.c    |   4 +-
>>  gdb/nat/fork-inferior.h    |   7 +-
>>  gdb/nat/linux-ptrace.c     |  86 +++++++++++++++++++++--
>>  gdb/nat/linux-ptrace.h     |  15 +++-
>>  11 files changed, 282 insertions(+), 18 deletions(-)
>>
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index e1bc8143e6..43f749c6f1 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -182,6 +182,9 @@ software in general.  We will miss him.
>>                                  @value{GDBN}
>>  * Operating System Information:: Getting additional information from
>>                                   the operating system
>> +* Linux kernel @code{ptrace} restrictions:: Restrictions sometimes
>> +                                            imposed by the Linux
>> +                                            kernel on @code{ptrace}
>>  * Trace File Format:: GDB trace file format
>>  * Index Section Format::        .gdb_index section format
>>  * Man Pages:: Manual pages
>> @@ -44682,6 +44685,141 @@ should contain a comma-separated list of cores that this process
>>  is running on.  Target may provide additional columns,
>>  which @value{GDBN} currently ignores.
>>  
>> +@node Linux kernel ptrace restrictions
>> +@appendix Linux kernel @code{ptrace} restrictions
>> +@cindex linux kernel ptrace restrictions, attach
>> +
>> +The @code{ptrace} system call is used by @value{GDBN} on GNU/Linux to,
>> +among other things, attach to a new or existing inferior in order to
>> +start debugging it.  Due to security concerns, some distributions and
>> +vendors disable or severily restrict the ability to perform these
>
> typo: severily -> severely

Fixed.

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

Yes, I meant "Pointer", thanks.  Fixed.

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

Indeed.  I'll update the comment there.

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

While writing this code I thought I remembered something like that, but
I wasn't sure.  Thanks for clarifying.  Updated.

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

OK.  Updated.

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

Ah, I hadn't realized that this existed.  Much better, thanks!

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

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.

--
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
On Thursday, August 29 2019, I wrote:

> On Thursday, August 29 2019, Pedro Alves wrote:
>
>>>   - It reads the contents of "/proc/sys/kernel/yama/ptrace_scope" and
>>>     checks if it's different than 0.
>>>
>>> For each of these checks, if it succeeds, the user will see a message
>>> informing about the restriction in place, and how it can be disabled.
>>> For example, if "deny_ptrace" is enabled, the user will see:
>>>
>>>   # gdb /bin/true
>>>   ...
>>>   Starting program: /usr/bin/true
>>>   warning: Could not trace the inferior process.
>>>   Error:
>>>   warning: ptrace: Permission denied
>>
>> Curious, that:
>>
>>  warning:
>>  Error:
>>  warning:
>>
>> looks a bit odd, specifically the "Error:" line.  Do you know where is
>> that coming from?
>
> Not offhand; I'll investigate and come back with the results.

So, the problem is with nat/fork-inferior.c:trace_start_error:

  void
  trace_start_error (const char *fmt, ...)
  {
    va_list ap;

    va_start (ap, fmt);
    warning ("Could not trace the inferior process.\nError: ");
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    vwarning (fmt, ap);
    va_end (ap);

    gdb_flush_out_err ();
    _exit (0177);
  }

  /* See nat/fork-inferior.h.  */

  void
  trace_start_error_with_name (const char *string, const char *append)
  {
    trace_start_error ("%s: %s%s", string, safe_strerror (errno), append);
  }


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?

--
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
In reply to this post by Sergio Durigan Junior
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.

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

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
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'

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

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

Eli Zaretskii
> 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.
12345