From patchwork Tue Nov 11 14:25:37 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 3654 Received: (qmail 27233 invoked by alias); 11 Nov 2014 14:25:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27221 invoked by uid 89); 11 Nov 2014 14:25:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 11 Nov 2014 14:25:43 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sABEPd5r002908 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 11 Nov 2014 09:25:40 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sABEPc1D014579; Tue, 11 Nov 2014 09:25:38 -0500 Message-ID: <54621C61.7050002@redhat.com> Date: Tue, 11 Nov 2014 14:25:37 +0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: Daniel Colascione , gdb-patches@sourceware.org Subject: Re: [PATCH] Warn users about mismatched PID namespaces References: <5451AB7E.40709@dancol.org> <545CBFEB.7000907@redhat.com> <545FDB3C.6060709@dancol.org> In-Reply-To: <545FDB3C.6060709@dancol.org> On 11/09/2014 09:23 PM, Daniel Colascione wrote: > On 11/07/2014 12:49 PM, Pedro Alves wrote: >> >> [1] BTW, could I interest in giving gdbserver/thread-db.c the >> same treatment? > > Do warnings from gdbserver even get propagated to remotes? To send output to GDB, we can use monitor_output, but I'd be happy with just calling warning on gdbserver too (goes to gdbserver's console). > I don't > see any obvious equivalent to the on-attach hook there. Maybe thread_db_init? >>> >> + >>> >> +char* >>> >> +linux_proc_pid_get_ns (pid_t pid, const char *ns) >>> >> +{ >>> >> + char buf[100]; >>> >> + char nsval[64]; >>> >> + int ret; >>> >> + snprintf (buf, sizeof (buf), "/proc/%d/ns/%s", (int) pid, ns); >> > >> > Use xsnprintf > Done. Looks like not. ;-) >>> +/* Return an opaque string identifying PID's NS namespace or NULL if >>> + * the information is unavailable. The returned string must be >>> + * released with xfree. */ >>> + >>> +extern char* linux_proc_pid_get_ns (pid_t pid, const char *ns); >> >> Space between char and *. > > What do you mean? Lots of other functions in GDB use the style I used in my patch. I mean the other char -- there's a space missing in "char*". That is, write instead: extern char *linux_proc_pid_get_ns (pid_t pid, const char *ns); > +char* > +linux_proc_pid_get_ns (pid_t pid, const char *ns) Here too, btw, should be: char * linux_proc_pid_get_ns (pid_t pid, const char *ns) > +static void > +check_pid_namespace_match () Should be: '(void)' not '()'. Those are different in C. I fixed these nits up for you, updated the ChangeLog entry and pushed the patch in. Thanks for the patch. From 015de6884f6fdebaffd4b7d4c7f14fb4d5fc0bb1 Mon Sep 17 00:00:00 2001 From: Daniel Colascione Date: Tue, 11 Nov 2014 14:18:23 +0000 Subject: [PATCH] Warn users about mismatched PID namespaces Linux supports multiple "PID namespaces". Processes in different PID namespaces have different views of the system process list. Sometimes, a single process can appear in more than one PID namespace, but with a different PID in each. When GDB and its target are in different PID namespaces, various features can break due to the mismatch between what the target believes its PID to be and what GDB believes its PID to be. The most visible broken functionality is thread enumeration silently failing. This patch explicitly warns users against trying to debug across PID namespaces. The patch introduced no new failures in my test suite run on an x86_64 installation of Ubuntu 14.10. It doesn't include a test: writing an automated test that exercises this code would be very involved because CLONE_NEWNS requires CAP_SYS_ADMIN; the easier way to reproduce the problem is to start a new lxc container. gdb/ 2014-11-11 Daniel Colascione Warn about cross-PID-namespace debugging. * nat/linux-procfs.h (linux_proc_pid_get_ns): New prototype. * nat/linux-procfs.c (linux_proc_pid_get_ns): New function. * linux-thread-db.c (check_pid_namespace_match): New function. (thread_db_inferior_created): Call it. --- gdb/ChangeLog | 8 ++++++++ gdb/linux-thread-db.c | 29 +++++++++++++++++++++++++++++ gdb/nat/linux-procfs.c | 19 +++++++++++++++++++ gdb/nat/linux-procfs.h | 6 ++++++ 4 files changed, 62 insertions(+) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 6a9b660..10321fd 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,11 @@ +2014-11-11 Daniel Colascione + + Warn about cross-PID-namespace debugging. + * nat/linux-procfs.h (linux_proc_pid_get_ns): New prototype. + * nat/linux-procfs.c (linux_proc_pid_get_ns): New function. + * linux-thread-db.c (check_pid_namespace_match): New function. + (thread_db_inferior_created): Call it. + 2014-11-10 Doug Evans * symmisc.c (print_objfile_statistics): Remove trailing whitespace. diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c index 352fac1..c49b567 100644 --- a/gdb/linux-thread-db.c +++ b/gdb/linux-thread-db.c @@ -1217,12 +1217,41 @@ thread_db_new_objfile (struct objfile *objfile) check_for_thread_db (); } +static void +check_pid_namespace_match (void) +{ + /* Check is only relevant for local targets targets. */ + if (target_can_run (¤t_target)) + { + /* If the child is in a different PID namespace, its idea of its + PID will differ from our idea of its PID. When we scan the + child's thread list, we'll mistakenly think it has no threads + since the thread PID fields won't match the PID we give to + libthread_db. */ + char *our_pid_ns = linux_proc_pid_get_ns (getpid (), "pid"); + char *inferior_pid_ns = linux_proc_pid_get_ns ( + ptid_get_pid (inferior_ptid), "pid"); + + if (our_pid_ns != NULL && inferior_pid_ns != NULL + && strcmp (our_pid_ns, inferior_pid_ns) != 0) + { + warning (_ ("Target and debugger are in different PID " + "namespaces; thread lists and other data are " + "likely unreliable")); + } + + xfree (our_pid_ns); + xfree (inferior_pid_ns); + } +} + /* This function is called via the inferior_created observer. This handles the case of debugging statically linked executables. */ static void thread_db_inferior_created (struct target_ops *target, int from_tty) { + check_pid_namespace_match (); check_for_thread_db (); } diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c index 30797da..00f2f08 100644 --- a/gdb/nat/linux-procfs.c +++ b/gdb/nat/linux-procfs.c @@ -113,3 +113,22 @@ linux_proc_pid_is_zombie (pid_t pid) { return linux_proc_pid_has_state (pid, "Z (zombie)"); } + +/* See linux-procfs.h declaration. */ + +char * +linux_proc_pid_get_ns (pid_t pid, const char *ns) +{ + char buf[100]; + char nsval[64]; + int ret; + xsnprintf (buf, sizeof (buf), "/proc/%d/ns/%s", (int) pid, ns); + ret = readlink (buf, nsval, sizeof (nsval)); + if (0 < ret && ret < sizeof (nsval)) + { + nsval[ret] = '\0'; + return xstrdup (nsval); + } + + return NULL; +} diff --git a/gdb/nat/linux-procfs.h b/gdb/nat/linux-procfs.h index d13fff7..5e2a9ea 100644 --- a/gdb/nat/linux-procfs.h +++ b/gdb/nat/linux-procfs.h @@ -40,4 +40,10 @@ extern int linux_proc_pid_is_stopped (pid_t pid); extern int linux_proc_pid_is_zombie (pid_t pid); +/* Return an opaque string identifying PID's NS namespace or NULL if + * the information is unavailable. The returned string must be + * released with xfree. */ + +extern char *linux_proc_pid_get_ns (pid_t pid, const char *ns); + #endif /* COMMON_LINUX_PROCFS_H */