From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-74.mimecast.com (us-smtp-delivery-74.mimecast.com [216.205.24.74]) by sourceware.org (Postfix) with ESMTP id AFD3A385BF9F for ; Fri, 27 Mar 2020 15:28:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AFD3A385BF9F Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-193-OSJMsMy8N4qBQhn2kydG6Q-1; Fri, 27 Mar 2020 11:28:24 -0400 X-MC-Unique: OSJMsMy8N4qBQhn2kydG6Q-1 Received: by mail-ed1-f71.google.com with SMTP id l4so8520452edw.23 for ; Fri, 27 Mar 2020 08:28:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=rJra5PazNqqEtdWrM627QBm12TGEZxQ6Y1AnPkmItC0=; b=OKTps6/9NB+8GzJsktWGEu5SudKOVu2lRo9VjzsKu0Yvw6ZFlynu/GhkRqkUFPGkxL 1lwlHQeGpO8+6aJbhY4rjGomifhk/DXIO75uRHTThqQtbKqre+njufUaHWChcTA2qaUK xWJrKJrezkq8t6IQS3ViEZFyXrJDhGlOKA1VuveqM7dP9MO3yNeS1bH/QNzAR7B1EieD 5ieNk59yhmzTb4oyht1e6Rz5yQdfn5vSOVW3KMimoHOjYid0aAk+u2iLASkc4aVJNOkl o6PYYOPG2RfR+95T/Gl/LzsDLpYMuC1j4XuYBqe9w8CYRmrZOcTcTsXveQNIV9kQzbrd plyg== X-Gm-Message-State: ANhLgQ3Y8lVYnK4qPOY/PecDzag7p60OWmNQqdxElRqIF3zL/YT+AHV0 eVSQIpxGKjBMF/UxxWyHoGPBvapo0Q3MpY5mgHHEzNxv+xsjbgGcvg0CjMpcvo/9kwLVTcpYYHZ QjS/L39ej8JRGJsjHpyfXQg== X-Received: by 2002:a17:906:505:: with SMTP id j5mr13198423eja.13.1585322902275; Fri, 27 Mar 2020 08:28:22 -0700 (PDT) X-Google-Smtp-Source: ADFU+vukrR5pN1FOBfJoGNifpvvHL2uTY6Bzrg2N7bps7OpxQkPHSVTd19O5yfxwHOR0yn931+KhYQ== X-Received: by 2002:a17:906:505:: with SMTP id j5mr13198358eja.13.1585322901425; Fri, 27 Mar 2020 08:28:21 -0700 (PDT) Received: from ?IPv6:2001:8a0:f909:7b00:56ee:75ff:fe8d:232b? ([2001:8a0:f909:7b00:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id a32sm915951edf.46.2020.03.27.08.28.19 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 27 Mar 2020 08:28:20 -0700 (PDT) Subject: Re: [PATCH v2 4/5] Extend GNU/Linux to check for ptrace error To: Sergio Durigan Junior , GDB Patches References: <20200226200542.746617-1-sergiodj@redhat.com> <20200317154719.2078283-1-sergiodj@redhat.com> <20200317154719.2078283-5-sergiodj@redhat.com> Cc: Tom Tromey From: Pedro Alves Message-ID: <675c031c-c115-6461-41e7-173efd13284f@redhat.com> Date: Fri, 27 Mar 2020 15:28:19 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20200317154719.2078283-5-sergiodj@redhat.com> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-23.1 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, UNSUBSCRIBE_BODY autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Mar 2020 15:28:35 -0000 On 3/17/20 3:47 PM, Sergio Durigan Junior via Gdb-patches wrote: > This patch implements the ptrace-errno-checking on the GNU/Linux > target (both native and remote). It builds on top of the previous > 'fork_inferior' extension patch. > > The idea is to provide a new 'traceme_fun' for each ptrace backend, > which will accept a new integer argument representing the write end of > the ptrace status pipe (that was created in 'fork_inferior'). This > function will invoke the actual tracing syscall (which is 'ptrace' in > this case), get its errno value and write it back via the pipe. You > can see examples of this new approach by looking at > 'inf_ptrace_me' (GDB) or 'linux_ptrace_fun' (gdbserver). > > The rest of the patch implements the necessary machinery to do > something useful with the errno information that we received from > 'ptrace'. > > In Fedora GDB, we carry the following patch: > > https://src.fedoraproject.org/rpms/gdb/blob/8ac06474ff1e2aa4920d14e0666b083eeaca8952/f/gdb-attach-fail-reasons-5of5.patch > > Its purpose is to try to detect a specific scenario where SELinux's > 'deny_ptrace' option is enabled, which prevents GDB from ptrace'ing in > order to debug the inferior (PTRACE_ATTACH and PTRACE_TRACEME will > fail with EACCES in this case). > > I like the idea of improving error detection and providing more > information to the user (a simple "Permission denied" can be really > frustrating), but I don't fully agree with the way the patch was > implemented: it makes GDB link against libselinux only for the sake of > consulting the 'deny_ptrace' setting, and then prints a warning if > ptrace failed and this setting is on. > > There is now a new function, 'linux_ptrace_restricted_fail_reason', > which does a few things to check what's wrong with ptrace: > > - It dlopen's "libselinux.so.1" and checks if the "deny_ptrace" > option is enabled. > > - It reads the contents of "/proc/sys/kernel/yama/ptrace_scope" and > checks if it's different than 0. > > For each of these checks, if it succeeds, the user will see a message > informing about the restriction in place, and how it can be disabled. > For example, if "deny_ptrace" is enabled, the user will see: > > # gdb /usr/bin/true > ... > (gdb) run > Starting program: /usr/bin/true > warning: Could not trace the inferior process. > warning: ptrace: Permission denied > > The SELinux 'deny_ptrace' option is enabled and preventing GDB > from using 'ptrace'. You can disable it by executing (as root): > > setsebool deny_ptrace off > > If you are debugging the inferior remotely, the ptrace restriction(s) must > be disabled in the target system (e.g., where GDBserver is running). > > In case "/proc/sys/kernel/yama/ptrace_scope" is > 0: > > # gdb /usr/bin/true > ... > (gdb) run > Starting program: /usr/bin/true > warning: Could not trace the inferior process. > warning: ptrace: Operation not permitted > > The Linux kernel's Yama ptrace scope is in effect, which can prevent > GDB from using 'ptrace'. You can disable it by executing (as root): > > echo 0 > /proc/sys/kernel/yama/ptrace_scope > > If you are debugging the inferior remotely, the ptrace restriction(s) must > be disabled in the target system (e.g., where GDBserver is running). > > If both restrictions are enabled, both messages will show up. > > This works for gdbserver as well, and actually fixes a latent bug I > found: when ptrace is restricted, gdbserver would hang due to an > unchecked ptrace call: > > # gdbserver :9988 /usr/bin/true > gdbserver: linux_ptrace_test_ret_to_nx: Cannot PTRACE_TRACEME: Operation not permitted > gdbserver: linux_ptrace_test_ret_to_nx: status 256 is not WIFSTOPPED! > gdbserver: linux_ptrace_test_ret_to_nx: failed to kill child pid 2668100 No such process > [ Here you would have to issue a C-c ] > > Now, you will see: > > # gdbserver :9988 /usr/bin/true > gdbserver: linux_ptrace_test_ret_to_nx: Cannot PTRACE_TRACEME: Permission denied > gdbserver: linux_ptrace_test_ret_to_nx: status 256 is not WIFSTOPPED! > gdbserver: linux_ptrace_test_ret_to_nx: failed to kill child pid 2766868 No such process > gdbserver: Could not trace the inferior process. > gdbserver: ptrace: Permission denied > > The SELinux 'deny_ptrace' option is enabled and preventing GDB > from using 'ptrace'. You can disable it by executing (as root): > > setsebool deny_ptrace off > > If you are debugging the inferior remotely, the ptrace restriction(s) need > to be disabled in the target system (e.g., where GDBserver is running). > 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. > If you are debugging the inferior remotely, the ptrace restriction(s) need > to be disabled in the target system (e.g., where GDBserver is running). > > This means that the series expands our documentation (in the next > patch) and creates a new appendix section named "Linux kernel ptrace > restrictions", with sub-sections for each possible restriction that > might be in place. > > Notice how, on every message, we instruct the user to "do the right > thing" if gdbserver is being used. This is because if the user > started gdbserver *before* any ptrace restriction was in place, and > then, for some reason, one or more restrictions get enabled, then the > error message will be displayed both on gdbserver *and* on the > connected GDB. Since the user will be piloting GDB, it's important to > explicitly say that the ptrace restrictions are enabled in the target, > where gdbserver is running. > > The current list of possible restrictions is: > > - SELinux's 'deny_ptrace' option (detected). > > - YAMA's /proc/sys/kernel/yama/ptrace_scope setting (detected). > > - seccomp on Docker containers (I couldn't find how to detect). > > It's important to mention that all of this is Linux-specific; as far > as I know, SELinux, YAMA and seccomp are Linux-only features. > > gdb/ChangeLog: > yyyy-mm-dd Sergio Durigan Junior > > * inf-ptrace.c: Include "nat/fork-inferior.h". > (inf_ptrace_me): New parameter "trace_errno_wpipe". Check > "ptrace" errno. > (inf_ptrace_target::attach): Rewrite to use > "inf_ptrace_target::ptrace_attach". > (inf_ptrace_target::ptrace_attach): New function, almost > identical to the previous "inf_ptrace_target::attach". > * inf-ptrace.h (struct inf_ptrace_target) : > New method. > * linux-nat.c: Include "nat/fork-inferior.h". > (attach_proc_task_lwp_callback): Call > "linux_ptrace_attach_fail_reason_lwp" instead of > "linux_ptrace_attach_fail_reason_string". > (linux_nat_target::attach): Save "ERRNO". Pass it to > "linux_ptrace_attach_fail_reason". > (_initialize_linux_nat): Set "trace_me_fail_reason". > * nat/linux-ptrace.c: Include "gdbsupport/gdb-dlfcn.h", > "nat/fork-inferior.h" and "gdbsupport/filestuff.h". > (selinux_ftype): New type. > (linux_ptrace_restricted_fail_reason): New function. > (linux_ptrace_attach_fail_reason_1): New function, renamed > from "linux_ptrace_attach_fail_reason". > (linux_ptrace_attach_fail_reason): New function. > (linux_ptrace_attach_fail_reason_lwp): Likewise. > (linux_ptrace_me_fail_reason): Likewise. > (errno_pipe): New variable. > (linux_child_function): Check "ptrace" errno. Send it through > the pipe. > (linux_check_ptrace_features): Initialize pipe. Check > "ptrace" errno sent through the pipe. > * nat/linux-ptrace.h (linux_ptrace_attach_fail_reason): New > function. > (linux_ptrace_attach_fail_reason_lwp): Likewise. > (linux_ptrace_me_fail_reason): Likewise. > * remote.c (extended_remote_target::attach): Check error > message on PACKET_ERROR. > (remote_target::extended_remote_run): Likewise. > > gdbserver/ChangeLog: > yyyy-mm-dd Sergio Durigan Junior > > * linux-low.cc (linux_ptrace_fun): New parameter > "trace_errno_wpipe". Check "ptrace" errno. > (attach_proc_task_lwp_callback): Call > "linux_ptrace_attach_fail_reason_lwp" instead of > "linux_ptrace_attach_fail_reason_string". > (linux_process_target::attach): Likewise. > (initialize_low): Set "trace_me_fail_reason". > * server.cc (handle_v_attach): Check if "attach_inferior" > succeeded. > (handle_v_run): Likewise. > * thread-db.cc (attach_thread): Call > "linux_ptrace_attach_fail_reason_lwp" instead of > "linux_ptrace_attach_fail_reason_string". > --- > gdb/inf-ptrace.c | 34 +++++++- > gdb/inf-ptrace.h | 2 + > gdb/linux-nat.c | 24 +++--- > gdb/nat/fork-inferior.c | 6 +- > gdb/nat/fork-inferior.h | 2 +- > gdb/nat/linux-ptrace.c | 178 ++++++++++++++++++++++++++++++++++++++-- > gdb/nat/linux-ptrace.h | 27 ++++-- > gdb/remote.c | 40 ++++++++- > gdbserver/linux-low.cc | 31 +++++-- > gdbserver/server.cc | 38 ++++++++- > gdbserver/thread-db.cc | 2 +- > 11 files changed, 339 insertions(+), 45 deletions(-) > > diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c > index db17a76d94..941f019709 100644 > --- a/gdb/inf-ptrace.c > +++ b/gdb/inf-ptrace.c > @@ -34,6 +34,7 @@ > #include "nat/fork-inferior.h" > #include "utils.h" > #include "gdbarch.h" > +#include "nat/fork-inferior.h" > > > > @@ -97,10 +98,23 @@ inf_ptrace_target::remove_fork_catchpoint (int pid) > /* Prepare to be traced. */ > > static void > -inf_ptrace_me (void) > +inf_ptrace_me (int trace_errno_wpipe) > { > /* "Trace me, Dr. Memory!" */ > - if (ptrace (PT_TRACE_ME, 0, (PTRACE_TYPE_ARG3) 0, 0) < 0) > + int ret = ptrace (PT_TRACE_ME, 0, (PTRACE_TYPE_ARG3) 0, 0); > + int ptrace_errno = ret < 0 ? errno : 0; > + > + try > + { > + write_trace_errno_to_pipe (trace_errno_wpipe, ptrace_errno); > + } > + catch (const gdb_exception &e) > + { > + warning ("%s", e.what ()); > + _exit (0177); > + } We've been avoiding try/catch in an async-signal-safe-only environment (between fork and exec/exit). This is another spot leaking in. AFAICT, all cases of write_trace_errno_to_pipe throwing end up caught be the immediate caller catching the error, warning and calling _exit. So how about doing the warning+_exit directly within write_trace_errno_to_pipe and thus avoid the try/catch? > + > + if (ret < 0) > trace_start_error_with_name ("ptrace"); Seems like errno was already lost when you get here, since write_trace_errno_to_pipe clobbers errno. BTW, you could do with a single ret < 0 check, I think? Like: if (ret < 0) { int ptrace_errno = errno; write_trace_errno_to_pipe (trace_errno_wpipe, ptrace_errno); errno = ptrace_errno; trace_start_error_with_name ("ptrace"); } Though you could make (and document) write_trace_errno_to_pipe preserve errno itself. I'd even consider removing its trace_errno parameter, resulting in this on the caller side: if (ret < 0) { write_trace_errno_to_pipe (trace_errno_wpipe); trace_start_error_with_name ("ptrace"); } Note how trace_start_error_with_name already uses the global errno, so it wouldn't be strange. > } > > @@ -185,6 +199,18 @@ inf_ptrace_target::mourn_inferior () > > void > inf_ptrace_target::attach (const char *args, int from_tty) > +{ > + errno = ptrace_attach (args, from_tty); > + if (errno != 0) > + perror_with_name (("ptrace")); > +} > + > +/* Attach to the process specified by ARGS. If FROM_TTY is non-zero, > + be chatty about it. Return ERRNO if the call to ptrace failed; 0 > + otherwise. */ This should also mention that we throw an error for other reasons. Something like: Returns ERRNO if the call to ptrace failed; 0 if ptrace succeeded. Throws an error if it fails for reasons other than a ptrace failure. > + > +int > +inf_ptrace_target::ptrace_attach (const char *args, int from_tty) > { > pid_t pid; > struct inferior *inf; > @@ -223,7 +249,7 @@ inf_ptrace_target::attach (const char *args, int from_tty) > errno = 0; > ptrace (PT_ATTACH, pid, (PTRACE_TYPE_ARG3)0, 0); > if (errno != 0) > - perror_with_name (("ptrace")); > + return errno; > #else > error (_("This system does not support attaching to a process")); > #endif > @@ -241,6 +267,8 @@ inf_ptrace_target::attach (const char *args, int from_tty) > set_executing (this, thr->ptid, true); > > unpusher.release (); > + > + return 0; > } > > #ifdef PT_GET_PROCESS_STATE > diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h > index dd0733736f..ca36ca3af3 100644 > --- a/gdb/inf-ptrace.h > +++ b/gdb/inf-ptrace.h > @@ -31,6 +31,8 @@ struct inf_ptrace_target : public inf_child_target > > void attach (const char *, int) override; > > + int ptrace_attach (const char *, int); > + > void detach (inferior *inf, int) override; > > void resume (ptid_t, int, enum gdb_signal) override; > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > index 81af83c4ac..cc044ee3ac 100644 > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c > @@ -31,6 +31,7 @@ > #include "nat/linux-ptrace.h" > #include "nat/linux-procfs.h" > #include "nat/linux-personality.h" > +#include "nat/fork-inferior.h" > #include "linux-fork.h" > #include "gdbthread.h" > #include "gdbcmd.h" > @@ -1136,7 +1137,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 ()); > @@ -1185,20 +1186,15 @@ linux_nat_target::attach (const char *args, int from_tty) > /* Make sure we report all signals during attach. */ > pass_signals ({}); > > - try > - { > - inf_ptrace_target::attach (args, from_tty); > - } > - catch (const gdb_exception_error &ex) > + int err = inf_ptrace_target::ptrace_attach (args, from_tty); > + > + if (err != 0) > { > 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, err); > > - if (!reason.empty ()) > - throw_error (ex.error, "warning: %s\n%s", reason.c_str (), > - ex.what ()); > - else > - throw_error (ex.error, "%s", ex.what ()); > + error (_("warning: ptrace: %s\n%s"), > + safe_strerror (err), reason.c_str ()); > } > > /* The ptrace base target adds the main thread with (pid,0,0) > @@ -4582,6 +4578,10 @@ Enables printf debugging output."), > sigemptyset (&blocked_mask); > > lwp_lwpid_htab_create (); > + > + /* Set the proper function to generate a message when ptrace > + fails. */ > + trace_me_fail_reason = linux_ptrace_me_fail_reason; > } > > > diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c > index 223ff44195..eb4c4625d7 100644 > --- a/gdb/nat/fork-inferior.c > +++ b/gdb/nat/fork-inferior.c > @@ -394,9 +394,9 @@ struct traceme_info > > This function will usually perform the call to whatever trace > function needed to start tracing the inferior, but will also > - write its errno value to TRACE_ERRNO_PIPE, so that > + write its errno value to TRACE_ERRNO_WPIPE, so that > fork_inferior_1 can check whether it suceeded. */ > - void (*traceme_fun_check) (int trace_errno_pipe); > + void (*traceme_fun_check) (int trace_errno_wpipe); I was surprised to see this renaming going on this patch. Why not name the variable like that in the previous patch, where it was introduced, to begin with? > } u; > }; > > @@ -626,7 +626,7 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs, > > pid_t > fork_inferior (const char *exec_file_arg, const std::string &allargs, > - char **env, void (*traceme_fun) (int trace_errno_pipe), > + char **env, void (*traceme_fun) (int trace_errno_wpipe), Ditto. Etc. > gdb::function_view init_trace_fun, > void (*pre_trace_fun) (), > const char *shell_file_arg, > diff --git a/gdb/nat/fork-inferior.h b/gdb/nat/fork-inferior.h > index b67215353f..3fbead2e33 100644 > --- a/gdb/nat/fork-inferior.h > +++ b/gdb/nat/fork-inferior.h > @@ -66,7 +66,7 @@ extern pid_t fork_inferior (const char *exec_file_arg, > extern pid_t fork_inferior (const char *exec_file_arg, > const std::string &allargs, > char **env, > - void (*traceme_fun) (int trace_errno_pipe), > + void (*traceme_fun) (int trace_errno_wpipe), > gdb::function_view init_trace_fun, > void (*pre_trace_fun) (), > const char *shell_file_arg, > diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c > index 5335d69092..b3fcf8bc07 100644 > --- a/gdb/nat/linux-ptrace.c > +++ b/gdb/nat/linux-ptrace.c > @@ -21,6 +21,9 @@ > #include "linux-procfs.h" > #include "linux-waitpid.h" > #include "gdbsupport/buffer.h" > +#include "gdbsupport/gdb-dlfcn.h" > +#include "nat/fork-inferior.h" > +#include "gdbsupport/filestuff.h" > #ifdef HAVE_SYS_PROCFS_H > #include > #endif > @@ -30,11 +33,93 @@ > of 0 means there are no supported features. */ > static int supported_ptrace_options = -1; > > -/* Find all possible reasons we could fail to attach PID and return these > - as a string. An empty string is returned if we didn't find any reason. */ > +typedef int (*selinux_ftype) (const char *); > > -std::string > -linux_ptrace_attach_fail_reason (pid_t pid) > +/* Helper function which checks if ptrace is probably restricted > + (i.e., if ERR is either EACCES or EPERM), and returns a string with > + possible workarounds. */ > + > +static std::string > +linux_ptrace_restricted_fail_reason (int err) > +{ > + if (err != EACCES && err != EPERM) > + { > + /* It just makes sense to perform the checks below if errno was > + either EACCES or EPERM. */ > + return {}; > + } > + > + std::string ret; > + gdb_dlhandle_up handle; > + > + try > + { > + handle = gdb_dlopen ("libselinux.so.1"); > + } > + catch (const gdb_exception_error &e) > + { > + } > + > + if (handle != nullptr) > + { > + selinux_ftype selinux_get_bool > + = (selinux_ftype) gdb_dlsym (handle, "security_get_boolean_active"); > + > + if (selinux_get_bool != NULL > + && (*selinux_get_bool) ("deny_ptrace") == 1) > + string_appendf (ret, > + _("\n\ > +The SELinux 'deny_ptrace' option is enabled and preventing GDB\n\ > +from using 'ptrace'. You can disable it by executing (as root):\n\ > +\n\ > + setsebool deny_ptrace off\n")); > + } > + > + gdb_file_up yama_ptrace_scope > + = gdb_fopen_cloexec ("/proc/sys/kernel/yama/ptrace_scope", "r"); > + > + if (yama_ptrace_scope != nullptr) > + { > + char yama_scope = fgetc (yama_ptrace_scope.get ()); > + > + if (yama_scope != '0') > + string_appendf (ret, > + _("\n\ > +The Linux kernel's Yama ptrace scope is in effect, which can prevent\n\ > +GDB from using 'ptrace'. You can disable it by executing (as root):\n\ > +\n\ > + echo 0 > /proc/sys/kernel/yama/ptrace_scope\n")); > + } > + > + if (ret.empty ()) > + { > + /* It wasn't possible to determine the exact reason for the > + ptrace error. Let's just emit a generic error message > + pointing the user to our documentation, where she can find > + instructions on how to try to diagnose the problem. */ > + ret = _("\n\ > +There might be restrictions preventing ptrace from working. Please see\n\ > +the appendix \"Linux kernel ptrace restrictions\" in the GDB documentation\n\ > +for more details."); > + } > + > + /* The user may be debugging remotely, so we have to warn that > + the instructions above should be performed in the target. */ > + string_appendf (ret, > + _("\n\ > +If you are debugging the inferior remotely, the ptrace restriction(s) must\n\ > +be disabled in the target system (e.g., where GDBserver is running).")); > + > + return ret; > +} > + > +/* Find all possible reasons we could fail to attach PID and return > + these as a string. An empty string is returned if we didn't find > + any reason. Helper for linux_ptrace_attach_fail_reason and > + linux_ptrace_attach_fail_reason_lwp. */ > + > +static std::string > +linux_ptrace_attach_fail_reason_1 (pid_t pid) > { > pid_t tracerpid = linux_proc_get_tracerpid_nowarn (pid); > std::string result; > @@ -56,10 +141,24 @@ linux_ptrace_attach_fail_reason (pid_t pid) > /* See linux-ptrace.h. */ > > std::string > -linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err) > +linux_ptrace_attach_fail_reason (pid_t pid, int err) > +{ > + std::string result = linux_ptrace_attach_fail_reason_1 (pid); > + std::string ptrace_restrict = linux_ptrace_restricted_fail_reason (err); > + > + if (!ptrace_restrict.empty ()) > + result += "\n" + ptrace_restrict; > + > + return result; > +} > + > +/* See linux-ptrace.h. */ > + > +std::string > +linux_ptrace_attach_fail_reason_lwp (ptid_t ptid, int err) > { > long lwpid = ptid.lwp (); > - std::string reason = linux_ptrace_attach_fail_reason (lwpid); > + std::string reason = linux_ptrace_attach_fail_reason_1 (lwpid); > > if (!reason.empty ()) > return string_printf ("%s (%d), %s", safe_strerror (err), err, > @@ -68,6 +167,14 @@ linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err) > return string_printf ("%s (%d)", safe_strerror (err), err); > } > > +/* See linux-ptrace.h. */ > + > +std::string > +linux_ptrace_me_fail_reason (int err) > +{ > + return linux_ptrace_restricted_fail_reason (err); > +} > + > #if defined __i386__ || defined __x86_64__ > > /* Address of the 'ret' instruction in asm code block below. */ > @@ -257,6 +364,12 @@ linux_ptrace_test_ret_to_nx (void) > #endif /* defined __i386__ || defined __x86_64__ */ > } > > +/* If the PTRACE_TRACEME call on linux_child_function errors, we need > + to be able to send ERRNO back to the parent so that it can check > + whether there are restrictions in place preventing ptrace from > + working. We do that with a pipe. */ > +static int errno_pipe[2]; > + We're missing a comment somewhere saying that we do that on the parent side to avoid doing non-async-signal-safe things in the child. Here might be a good place, but on the gdb side we could use some comment about it too. > /* Helper function to fork a process and make the child process call > the function FUNCTION, passing CHILD_STACK as parameter. > > @@ -321,7 +434,30 @@ linux_grandchild_function (void *child_stack) > static int > linux_child_function (void *child_stack) > { > - ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0, (PTRACE_TYPE_ARG4) 0); > + /* Close read end. */ > + close (errno_pipe[0]); > + > + int ret = ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0, > + (PTRACE_TYPE_ARG4) 0); > + int ptrace_errno = ret < 0 ? errno : 0; > + > + /* Write ERRNO to the pipe, even if it's zero, and close the writing > + end of the pipe. */ > + try > + { > + write_trace_errno_to_pipe (errno_pipe[1], ptrace_errno); > + } > + catch (const gdb_exception &e) > + { > + warning ("%s", e.what ()); > + _exit (0177); > + } Here's another spot that should do away with try/catch. > + > + close (errno_pipe[1]); > + > + if (ret != 0) > + trace_start_error_with_name ("ptrace"); > + > kill (getpid (), SIGSTOP); > > /* Fork a grandchild. */ > @@ -346,12 +482,40 @@ linux_check_ptrace_features (void) > /* Initialize the options. */ > supported_ptrace_options = 0; > > + /* Initialize our pipe. */ > + if (gdb_pipe_cloexec (errno_pipe) < 0) > + perror_with_name ("gdb_pipe_cloexec"); > + > /* Fork a child so we can do some testing. The child will call > linux_child_function and will get traced. The child will > eventually fork a grandchild so we can test fork event > reporting. */ > child_pid = linux_fork_to_function (NULL, linux_child_function); > > + /* We don't need the write end of the pipe anymore. */ > + close (errno_pipe[1]); > + > + try > + { > + /* Check whether 'ptrace (PTRACE_ME, ...)' failed when being > + invoked by the child. If it did, we might get the > + possible reason for it as the exception message. */ > + check_child_trace_me_errno (errno_pipe[0]); This is assuming that close doesn't clober errno, which in general is not garanteed: https://pubs.opengroup.org/onlinepubs/9699919799/functions/errno.html#tag_16_110 "The setting of errno after a successful call to a function is unspecified unless the description of that function specifies that errno shall not be modified." A quick web search finds this for example: https://git.furworks.de/opensourcemirror/git/commit/06121a0a8328c8aaa7a023cf6ebb142e9dc2b45c > + } > + catch (const gdb_exception &e) > + { > + /* Close the pipe so we don't leak fd's. */ fd's -> fds > + close (errno_pipe[0]); > + > + /* A failure here means that PTRACE_ME failed, which means that > + GDB/gdbserver will most probably not work correctly. If we > + want to be pedantic, we could just call 'exit' here. > + However, let's just re-throw the exception. */ > + throw; > + } > + > + close (errno_pipe[0]); > + > ret = my_waitpid (child_pid, &status, 0); > if (ret == -1) > perror_with_name (("waitpid")); > diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h > index 65568301f2..7cb77114ca 100644 > --- a/gdb/nat/linux-ptrace.h > +++ b/gdb/nat/linux-ptrace.h > @@ -176,12 +176,27 @@ struct buffer; > # define TRAP_HWBKPT 4 > #endif > > -extern std::string linux_ptrace_attach_fail_reason (pid_t pid); > - > -/* Find all possible reasons we could have failed to attach to PTID > - and return them as a string. ERR is the error PTRACE_ATTACH failed > - with (an errno). */ > -extern std::string linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err); > +/* Find all possible reasons we could fail to attach PID and return > + these as a string. An empty string is returned if we didn't find > + any reason. If ERR is EACCES or EPERM, we also add a warning about > + possible restrictions to use ptrace. */ > +extern std::string linux_ptrace_attach_fail_reason (pid_t pid, int err); > + > +/* Find all possible reasons we could have failed to attach to PID's > + LWPID and return them as a string. ERR is the error PTRACE_ATTACH > + failed with (an errno). Unlike linux_ptrace_attach_fail_reason, > + this function should be used when attaching to an LWP other than > + the leader; it does not warn about ptrace restrictions. */ > +extern std::string linux_ptrace_attach_fail_reason_lwp (ptid_t pid, int err); > + > +/* When the call to 'ptrace (PTRACE_TRACEME...' fails, and we have > + already forked, this function can be called in order to try to > + obtain the reason why ptrace failed. ERR should be the ERRNO value > + returned by ptrace. > + > + This function will return a 'std::string' containing the fail > + reason, or an empty string otherwise. */ > +extern std::string linux_ptrace_me_fail_reason (int err); > > extern void linux_ptrace_init_warnings (void); > extern void linux_check_ptrace_features (void); > diff --git a/gdb/remote.c b/gdb/remote.c > index 0f78b1be1b..aacbdf1984 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -5882,9 +5882,26 @@ extended_remote_target::attach (const char *args, int from_tty) > break; > case PACKET_UNKNOWN: > error (_("This target does not support attaching to a process")); > + case PACKET_ERROR: > + { > + std::string errmsg = rs->buf.data (); This is taking a deep copy which seems unnecessary. The code would look almost the same without it. Like: const char *errmsg = rs->buf.data (); /* Check if we have a specific error (i.e., not a generic "E01") coming from the target. If there is, we print it here. */ if (startswith (errmsg, "E.")) { /* Get rid of the "E." prefix. */ errmsg += 2; } error (_("Attaching to %s failed%s%s"), target_pid_to_str (ptid_t (pid)).c_str (), errmsg != '\0' ? "\n" : "", errmsg); > + > + /* Check if we have a specific error (i.e., not a generic > + "E01") coming from the target. If there is, we print it > + here. */ > + if (startswith (errmsg.c_str (), "E.")) > + { > + /* Get rid of the "E." prefix. */ > + errmsg.erase (0, 2); > + } > + > + error (_("Attaching to %s failed%s%s"), > + target_pid_to_str (ptid_t (pid)).c_str (), > + !errmsg.empty () ? "\n" : "", > + errmsg.c_str ()); > + } > default: > - error (_("Attaching to %s failed"), > - target_pid_to_str (ptid_t (pid)).c_str ()); > + gdb_assert_not_reached (_("bad switch")); > } > > set_current_inferior (remote_add_inferior (false, pid, 1, 0)); > @@ -10024,8 +10041,23 @@ remote_target::extended_remote_run (const std::string &args) > error (_("Running the default executable on the remote target failed; " > "try \"set remote exec-file\"?")); > else > - error (_("Running \"%s\" on the remote target failed"), > - remote_exec_file); > + { > + std::string errmsg = rs->buf.data (); > + > + /* Check if we have a specific error (i.e., not a generic > + "E01") coming from the target. If there is, we print it > + here. */ > + if (startswith (errmsg.c_str (), "E.")) > + { > + /* Get rid of the "E." prefix. */ > + errmsg.erase (0, 2); > + } > + > + error (_("Running \"%s\" on the remote target failed%s%s"), > + remote_exec_file, > + !errmsg.empty () ? "\n" : "", > + errmsg.c_str ()); Ditto. > + } > default: > gdb_assert_not_reached (_("bad switch")); > } > diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc > index 2872bc78da..42283802dd 100644 > --- a/gdbserver/linux-low.cc > +++ b/gdbserver/linux-low.cc > @@ -968,10 +968,24 @@ add_lwp (ptid_t ptid) > actually initiating the tracing of the inferior. */ > > static void > -linux_ptrace_fun () > +linux_ptrace_fun (int ptrace_errno_wpipe) > { > - if (ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0, > - (PTRACE_TYPE_ARG4) 0) < 0) > + int ret = ptrace (PTRACE_TRACEME, 0, (PTRACE_TYPE_ARG3) 0, > + (PTRACE_TYPE_ARG4) 0); > + int ptrace_errno = ret < 0 ? errno : 0; > + > + try > + { > + write_trace_errno_to_pipe (ptrace_errno_wpipe, ptrace_errno); > + } > + catch (const gdb_exception &e) > + { > + warning ("%s", e.what ()); > + _exit (0177); > + } Another spot that could do without try/catch. > + > + errno = ptrace_errno; > + if (ret < 0) > trace_start_error_with_name ("ptrace"); > > if (setpgid (0, 0) < 0) > @@ -1170,7 +1184,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,8 +1216,8 @@ linux_process_target::attach (unsigned long pid) > { > remove_process (proc); > > - std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err); > - error ("Cannot attach to process %ld: %s", pid, reason.c_str ()); > + std::string reason = linux_ptrace_attach_fail_reason (pid, err); > + error (_("Cannot attach to process %ld: %s"), pid, reason.c_str ()); > } > > /* Don't ignore the initial SIGSTOP if we just attached to this > @@ -7552,5 +7566,10 @@ initialize_low (void) > > initialize_low_arch (); > > + /* Initialize the 'trace_me_fail_reason' function pointer. We will > + use this to determine the reason for possible failures when > + invoking 'ptrace (PTRACE_ME, ...)'. */ > + trace_me_fail_reason = linux_ptrace_me_fail_reason; > + > linux_check_ptrace_features (); > } > diff --git a/gdbserver/server.cc b/gdbserver/server.cc > index 43962adc86..003385f42a 100644 > --- a/gdbserver/server.cc > +++ b/gdbserver/server.cc > @@ -2892,9 +2892,31 @@ handle_v_attach (char *own_buf) > { > client_state &cs = get_client_state (); > int pid; > + int ret; > > pid = strtol (own_buf + 8, NULL, 16); > - if (pid != 0 && attach_inferior (pid) == 0) > + > + if (pid <= 0) > + { > + write_enn (own_buf); > + return 0; > + } > + > + try > + { > + /* Attach to the specified PID. This function can throw, so we > + make sure to catch the exception and send it (as an error > + packet) back to GDB. */ > + ret = attach_inferior (pid); > + } > + catch (const gdb_exception_error &e) > + { > + fprintf (stderr, "%s\n", e.what ()); > + snprintf (own_buf, PBUFSIZ, "E.%s", e.what ()); > + return 0; > + } > + > + if (ret == 0) > { > /* Don't report shared library events after attaching, even if > some libraries are preloaded. GDB will always poll the > @@ -3030,7 +3052,19 @@ handle_v_run (char *own_buf) > free_vector_argv (program_args); > program_args = new_argv; > > - target_create_inferior (program_path.get (), program_args); > + try > + { > + /* Create the inferior. This function can throw, so we make > + sure to catch the exception and send it (as an error packet) > + back to GDB. */ > + target_create_inferior (program_path.get (), program_args); > + } > + catch (const gdb_exception_error &e) > + { > + fprintf (stderr, "%s\n", e.what ()); > + snprintf (own_buf, PBUFSIZ, "E.%s", e.what ()); > + return 0; > + } > > if (cs.last_status.kind == TARGET_WAITKIND_STOPPED) > { > diff --git a/gdbserver/thread-db.cc b/gdbserver/thread-db.cc > index 2bb6d28820..60ceb7b663 100644 > --- a/gdbserver/thread-db.cc > +++ b/gdbserver/thread-db.cc > @@ -224,7 +224,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 ()); > -- Thanks, Pedro Alves