[PATCH] Warn users about mismatched PID namespaces

[PATCH] Warn users about mismatched PID namespaces

[Daniel Colascione]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

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.

2014-10-30  Daniel Colascione  <dancol@dancol.org>

	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 (thread_db_inferior_created): Check for
	mismatched PID namespaces.

diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 352fac1..4089417 100644
- --- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile)
 static void
 thread_db_inferior_created (struct target_ops *target, int from_tty)
 {
+  /* 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);
+
   check_for_thread_db ();
 }

diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index 30797da..8efccba 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;
+  snprintf (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..e4f301f 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 */

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCAAGBQJUUat+AAoJEN4WImmbpWBlj78P/i090ErNhO9EwA5BbcpEnRsz
J9Zrjyr2TPKvDyeMB7ZWqU+YlXRbG0vzVoagyMxek6Qru2n6EnVPJIypriefim1D
scDQ0qkNPoK67T9/14O5xHVEgDl6ABjWV+3JWD7XXFS2YPpd2zQnCioW5dFIbtcw
0Ezj6TmBtOTHOXRGUTKi8USYUtF8U2qnH/Nf+p0TOGOJqMGJHo8LeDqWSTE9kIE6
z7xjsJKuzMrwnprtDKUH6rqlgjjQH9uTlF5XZ6hTV98+CHGPkB3NyAEYytW0FF0y
yTKqjmb+8QYMNZ75fJuP7PAvnbOU6vAfjZZu4CIoNDgDwCZcPtp3NC9TNxmZK5FV
xk3gEBBY0IxYNEqH8qiV2irK3OAgotxUdJSMLQEcOoId40bIIs4rkGbpBq4jW+2A
Rkfrman5L+5zHWRyamzmd/8aoh3zu+mNDeXzzc0AIKcw0hXv+aBu+eQ7keajvoD7
yAqCTzit3uaYsxYbPNjta1d39uB/xiCrs1Vm8ZKJuRxqcHB2gw89+QC+NEEWohkd
lS3rSyP4r65pHCQOdRnKu401/uJi6ouwxPb7mbw+MXFXi369U7vKoZ82uuv6N3uk
TnhnC8eQIZijflAkG+OlZQyi+EuzwPC++e9Ugq15vtyqBtCwnDlh+AWTvWVosKDB
rzNT57A9vmTcRg3CNXz/
=NLmY
-----END PGP SIGNATURE-----

Re: [PATCH] Warn users about mismatched PID namespaces

[Pedro Alves]

On 10/30/2014 03:07 AM, Daniel Colascione wrote:

> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
> index 352fac1..4089417 100644
> - --- a/gdb/linux-thread-db.c
> +++ b/gdb/linux-thread-db.c
> @@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile)
>  static void
>  thread_db_inferior_created (struct target_ops *target, int from_tty)
>  {
> +  /* 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.  */

Why not give libthread_db the right PID then?

Thanks,
Pedro Alves

Re: [PATCH] Warn users about mismatched PID namespaces

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

On 10/30/2014 12:23 PM, Pedro Alves wrote:
> On 10/30/2014 03:07 AM, Daniel Colascione wrote:
> 
>> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
>> index 352fac1..4089417 100644
>> - --- a/gdb/linux-thread-db.c
>> +++ b/gdb/linux-thread-db.c
>> @@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile)
>>  static void
>>  thread_db_inferior_created (struct target_ops *target, int from_tty)
>>  {
>> +  /* 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.  */
> 
> Why not give libthread_db the right PID then?

How do you suggest find it? There's some talk on LKML of adding the necessary system call, but it's not in-tree yet.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Warn users about mismatched PID namespaces

[Pedro Alves]

On 10/30/2014 12:32 PM, Daniel Colascione wrote:
> On 10/30/2014 12:23 PM, Pedro Alves wrote:
>> On 10/30/2014 03:07 AM, Daniel Colascione wrote:
>>
>>> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
>>> index 352fac1..4089417 100644
>>> - --- a/gdb/linux-thread-db.c
>>> +++ b/gdb/linux-thread-db.c
>>> @@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile)
>>>  static void
>>>  thread_db_inferior_created (struct target_ops *target, int from_tty)
>>>  {
>>> +  /* 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.  */
>>
>> Why not give libthread_db the right PID then?
> 
> How do you suggest find it? 

Isn't it visible somewhere in /proc ?

> There's some talk on LKML of adding the necessary system call, but it's not in-tree yet.

Do you have a url handy?

Thanks,
Pedro Alves

Re: [PATCH] Warn users about mismatched PID namespaces

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

On 10/30/2014 12:49 PM, Pedro Alves wrote:
> On 10/30/2014 12:32 PM, Daniel Colascione wrote:
>> On 10/30/2014 12:23 PM, Pedro Alves wrote:
>>> On 10/30/2014 03:07 AM, Daniel Colascione wrote:
>>>
>>>> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
>>>> index 352fac1..4089417 100644
>>>> - --- a/gdb/linux-thread-db.c
>>>> +++ b/gdb/linux-thread-db.c
>>>> @@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile)
>>>>  static void
>>>>  thread_db_inferior_created (struct target_ops *target, int from_tty)
>>>>  {
>>>> +  /* 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.  */
>>>
>>> Why not give libthread_db the right PID then?
>>
>> How do you suggest find it? 
> 
> Isn't it visible somewhere in /proc ?

Not AFAICT, but maybe I overlooked something.

> 
>> There's some talk on LKML of adding the necessary system call, but it's not in-tree yet.
> 
> Do you have a url handy?

https://lwn.net/Articles/602987/


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Warn users about mismatched PID namespaces

[Pedro Alves]

On 10/30/2014 12:51 PM, Daniel Colascione wrote:
> On 10/30/2014 12:49 PM, Pedro Alves wrote:
>> On 10/30/2014 12:32 PM, Daniel Colascione wrote:
>>> On 10/30/2014 12:23 PM, Pedro Alves wrote:
>>>> On 10/30/2014 03:07 AM, Daniel Colascione wrote:
>>>>
>>>>> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
>>>>> index 352fac1..4089417 100644
>>>>> - --- a/gdb/linux-thread-db.c
>>>>> +++ b/gdb/linux-thread-db.c
>>>>> @@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile)
>>>>>  static void
>>>>>  thread_db_inferior_created (struct target_ops *target, int from_tty)
>>>>>  {
>>>>> +  /* 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.  */
>>>>
>>>> Why not give libthread_db the right PID then?
>>>
>>> How do you suggest find it? 
>>
>> Isn't it visible somewhere in /proc ?
> 
> Not AFAICT, but maybe I overlooked something.

Oh well...  Fine with me to add a warning then.  I'd appreciate
that the comment in the code mentioned that that's no way to
retrieve the right PID.  I see a couple minor formatting issues in
the patch, but nothing major.  Do you have your copyright assignment
for GDB on file?  Seems you're only covered for emacs atm,
unfortunately.

> 
>>
>>> There's some talk on LKML of adding the necessary system call, but it's not in-tree yet.
>>
>> Do you have a url handy?
> 
> https://lwn.net/Articles/602987/

Thanks.

Thanks,
Pedro Alves

Re: [PATCH] Warn users about mismatched PID namespaces

[Pedro Alves]

On 10/30/2014 01:11 PM, Pedro Alves wrote:
> On 10/30/2014 12:51 PM, Daniel Colascione wrote:
>> On 10/30/2014 12:49 PM, Pedro Alves wrote:

>>>>> Why not give libthread_db the right PID then?
>>>>
>>>> How do you suggest find it? 
>>>
>>> Isn't it visible somewhere in /proc ?
>>
>> Not AFAICT, but maybe I overlooked something.
> 
> Oh well...  

...

>>>> There's some talk on LKML of adding the necessary system call, but it's not in-tree yet.
>>>
>>> Do you have a url handy?
>>
>> https://lwn.net/Articles/602987/
> 
> Thanks.

BTW, this sounds like something we should add to the kernel wish list:

https://sourceware.org/gdb/wiki/LinuxKernelWishList

Thanks,
Pedro Alves

Re: [PATCH] Warn users about mismatched PID namespaces

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 261 bytes --]

On 10/30/2014 01:11 PM, Pedro Alves wrote:
> Do you have your copyright assignment
> for GDB on file?  Seems you're only covered for emacs atm,
> unfortunately.

Working on it. I'm looking forward to using the new-ish GPG process. What do I need to do?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Warn users about mismatched PID namespaces

[Pedro Alves]

On 10/30/2014 03:41 PM, Daniel Colascione wrote:
> On 10/30/2014 01:11 PM, Pedro Alves wrote:
>> Do you have your copyright assignment
>> for GDB on file?  Seems you're only covered for emacs atm,
>> unfortunately.
> 
> Working on it. I'm looking forward to using the new-ish GPG process. What do I need to do?

I think that process bootstraps like usual.  Fill out:

 http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future

and send it to assign@gnu.org.  They'll let you know how to proceed.

Thanks,
Pedro Alves

Re: [PATCH] Warn users about mismatched PID namespaces

[Pedro Alves]

On 10/30/2014 01:11 PM, Pedro Alves wrote:
> On 10/30/2014 12:51 PM, Daniel Colascione wrote:
>> On 10/30/2014 12:49 PM, Pedro Alves wrote:
>>> On 10/30/2014 12:32 PM, Daniel Colascione wrote:
>>>> On 10/30/2014 12:23 PM, Pedro Alves wrote:
>>>>> On 10/30/2014 03:07 AM, Daniel Colascione wrote:
>>>>>
>>>>>> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
>>>>>> index 352fac1..4089417 100644
>>>>>> - --- a/gdb/linux-thread-db.c
>>>>>> +++ b/gdb/linux-thread-db.c
>>>>>> @@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile)
>>>>>>  static void
>>>>>>  thread_db_inferior_created (struct target_ops *target, int from_tty)
>>>>>>  {
>>>>>> +  /* 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.  */
>>>>>
>>>>> Why not give libthread_db the right PID then?
>>>>
>>>> How do you suggest find it? 
>>>
>>> Isn't it visible somewhere in /proc ?
>>
>> Not AFAICT, but maybe I overlooked something.
> 
> Oh well...

FYI, looks like exposing the info in /proc is in the works
as well.  I just stumbled uppon this:

  https://lkml.org/lkml/2014/11/5/174

Thanks,
Pedro Alves

Re: [PATCH] Warn users about mismatched PID namespaces

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

On 11/05/2014 10:55 AM, Pedro Alves wrote:
> On 10/30/2014 01:11 PM, Pedro Alves wrote:
>> On 10/30/2014 12:51 PM, Daniel Colascione wrote:
>>> On 10/30/2014 12:49 PM, Pedro Alves wrote:
>>>> On 10/30/2014 12:32 PM, Daniel Colascione wrote:
>>>>> On 10/30/2014 12:23 PM, Pedro Alves wrote:
>>>>>> On 10/30/2014 03:07 AM, Daniel Colascione wrote:
>>>>>>
>>>>>>> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
>>>>>>> index 352fac1..4089417 100644
>>>>>>> - --- a/gdb/linux-thread-db.c
>>>>>>> +++ b/gdb/linux-thread-db.c
>>>>>>> @@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile)
>>>>>>>  static void
>>>>>>>  thread_db_inferior_created (struct target_ops *target, int from_tty)
>>>>>>>  {
>>>>>>> +  /* 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.  */
>>>>>>
>>>>>> Why not give libthread_db the right PID then?
>>>>>
>>>>> How do you suggest find it? 
>>>>
>>>> Isn't it visible somewhere in /proc ?
>>>
>>> Not AFAICT, but maybe I overlooked something.
>>
>> Oh well...
> 
> FYI, looks like exposing the info in /proc is in the works
> as well.  I just stumbled uppon this:
> 
>   https://lkml.org/lkml/2014/11/5/174

That's good to hear. It won't help users of current kernels though.

Anyway, my paperwork just went through. Is there anything I need to do now?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Warn users about mismatched PID namespaces

[Pedro Alves]

On 10/30/2014 03:07 AM, Daniel Colascione wrote:

> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
> index 352fac1..4089417 100644
> - --- a/gdb/linux-thread-db.c
> +++ b/gdb/linux-thread-db.c
> @@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile)
>  static void
>  thread_db_inferior_created (struct target_ops *target, int from_tty)
>  {

This hook is called even if the current target isn't a native process.
E.g., you may have loaded a core, in which case looking at
getpid or /proc doesn't make sense.  Or you may be debugging with
"target sim", or a remote process with gdbserver [1], etc.

We need this same check that thread_db_load does:

  /* Don't attempt to use thread_db for remote targets.  */
  if (!(target_can_run (&current_target) || core_bfd))
    return 0;

[1] BTW, could I interest in giving gdbserver/thread-db.c the
    same treatment?

> +  /* 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 &&

Put '&&' at the beginning of the next line.

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

Please factor this new code to a function; Having it in a function
makes it easier to move the caller around if necessary.

> +
>    check_for_thread_db ();
>  }
> 
> diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
> index 30797da..8efccba 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;
> +  snprintf (buf, sizeof (buf), "/proc/%d/ns/%s", (int) pid, ns);

Use xsnprintf .

> 
> +/* 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 *.

Thanks,
Pedro Alves

Re: [PATCH] Warn users about mismatched PID namespaces

[Pedro Alves]

On 11/06/2014 09:12 PM, Daniel Colascione wrote:
> On 11/05/2014 10:55 AM, Pedro Alves wrote:

>> FYI, looks like exposing the info in /proc is in the works
>> as well.  I just stumbled uppon this:
>>
>>   https://lkml.org/lkml/2014/11/5/174
> 
> That's good to hear. It won't help users of current kernels though.

For sure.

> Anyway, my paperwork just went through. 

Thanks.

> Is there anything I need to do now?

Address the review comments I just sent.  :-)

Thanks,
Pedro Alves

Re: [PATCH] Warn users about mismatched PID namespaces

[Daniel Colascione]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Second version follows.

On 11/07/2014 12:49 PM, Pedro Alves wrote:
> On 10/30/2014 03:07 AM, Daniel Colascione wrote:
> 
>> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
>> index 352fac1..4089417 100644
>> - --- a/gdb/linux-thread-db.c
>> +++ b/gdb/linux-thread-db.c
>> @@ -1223,6 +1223,25 @@ thread_db_new_objfile (struct objfile *objfile)
>>  static void
>>  thread_db_inferior_created (struct target_ops *target, int from_tty)
>>  {
> 
> This hook is called even if the current target isn't a native process.
> E.g., you may have loaded a core, in which case looking at
> getpid or /proc doesn't make sense.  Or you may be debugging with
> "target sim", or a remote process with gdbserver [1], etc.
> 
> We need this same check that thread_db_load does:
> 
>   /* Don't attempt to use thread_db for remote targets.  */
>   if (!(target_can_run (&current_target) || core_bfd))
>     return 0;
> 
> [1] BTW, could I interest in giving gdbserver/thread-db.c the
>     same treatment?

Do warnings from gdbserver even get propagated to remotes? I don't
see any obvious equivalent to the on-attach hook there.

>> +  /* 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 &&
> 
> Put '&&' at the beginning of the next line.

Done

> 
>> +      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);
> 
> Please factor this new code to a function; Having it in a function
> makes it easier to move the caller around if necessary.

Done.

> 
>> +
>>    check_for_thread_db ();
>>  }
>>
>> diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
>> index 30797da..8efccba 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;
>> +  snprintf (buf, sizeof (buf), "/proc/%d/ns/%s", (int) pid, ns);
> 
> Use xsnprintf

Done.

>>
>> +/* 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.


diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 352fac1..bd3635e 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 ()
+{
+  /* Check is only relevant for local targets targets.  */
+  if (target_can_run (&current_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..8efccba 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;
+  snprintf (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..e4f301f 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 */

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCAAGBQJUX9s8AAoJEN4WImmbpWBlApwP/2KmKdnBIh55ZDUggMOL63Ui
uFDcuccoQUH+WiLgVw36SJFLFOlDl4wab3ipg37eUiChR/iBXnAuBqHRIVTO5VfF
AkjXRAJw5obNcW0b62aD645gjQd4hOsvaYRDcuV6W+vbn1FIPxRxNGAHwtZQ7PNE
06vJu//OWbjQZVogGgVojDK7j1oWEm66nR2Se5UPOk+ddxqoFDtSCNAiutnhlstt
w4+Dh3NCDSJUnoL3qPQf8ex8qymojvwshTvJLOMVX7XzXnuGNXZg/8Au0SHAnTeI
CEwdrgw+SG3vXdp9BnzPn2EYzIiyu6lAfHp4HoRojmmMXhtk2ES+qg/W/Ty6Os6Q
wkPVa5W5TOygh6Ux213LyjJlnYl3fOFa3Hmg20Yu3AwmvIy4UDsgQjBqUouDhv2n
2FZPXZ+4XiRuK4zuPrXgfdP9ywYpshY/Yxaj4a+blHOMkViu3G8sWxKLDa90juAl
xjiYuevQHmTe6EOomuIfC/gzKszQhz9SnCpycgCtB9UCruHrgoFnHJYQwi760ynf
t7anmvUV7jWIUrH+2OQBEa+kEl3uM9fkQJa6x71bMfH6Ob8MYq7ciJv78uM38nfo
3KyGSsb4RR6aiLyLsBJIesmLfYKxMGm/uGd4IJJrvbf3mCH2YoNLPZNPCSKrUXfv
tD41GDUaXGSEYKBHYkSg
=bck+
-----END PGP SIGNATURE-----

Re: [PATCH] Warn users about mismatched PID namespaces

[Pedro Alves]

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 <dancol@dancol.org>
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  <dancol@dancol.org>

	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  <dancol@dancol.org>
+
+	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  <xdje42@gmail.com>
 
 	* 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 (&current_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 */
-- 
1.9.3