public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* /proc/$PID/sched does not take PID namespace into account
@ 2013-09-03 14:22 Emmanuel Deloget
  2013-09-09 11:01 ` Peter Zijlstra
  0 siblings, 1 reply; 5+ messages in thread
From: Emmanuel Deloget @ 2013-09-03 14:22 UTC (permalink / raw)
  To: [ML] linux-kernel; +Cc: Emmanuel Deloget, Ingo Molnar, Peter Zijlstra

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

Hello,

(please CC me when answering this mail ; and sorry for my borken
English).

I noticed that when a process is executed in a PID namespace it can
still find his real PID by parsing /proc/$PID_IN_NS/sched.

(in this example I created a new PID namespace and ran /bin/bash at
PID 1)

  # uname -a
  Linux my-thinkpad 3.9-1-amd64 #1 SMP Debian 3.9.6-1 x86_64 GNU/Linux
  #
  # pidof bash
  1
  # head -n 1 /proc/1/sched
  bash (14957, #threads: 1)
  # cat /proc/1/stat
  1 (bash) S 0 1 0 34823 220 4202752 11359 (...)

In the root PID namespace

  # pidof bash
  (...) 14957 (...)
  # head -n 1 /proc/14957/sched
  bash (14957, #threads: 1)
  # cat /proc/14957/stat
  14957 (bash) S 14956 14957 23465 34823 14957 4202752 11386 (...)

The principle of least surprise tells me that if my PID is n in a
particular PID namespace then every stat or debug info should tell
me that my PID is n (i.e. I should not be able to get the real PID
of my program). This is a consistency issue: if I get different
information from different sources I may not be able to tell which
one is the right one.

The issue (if this is really an issue) lies in kernel/sched/debug.c,
function proc_sched_show_task(). The code says [1]:

    SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
                        get_nr_threads(p));

I understand that you're not supposed to use the content of a /proc
file that has been generated by a kernel source file named "debug.c"
in a production environment yet several distributions out there seems
to enable this by default (at least the Debian distribution I'm using
does it).

I see a few options:

* either it's a bug and it should be corrected (I'm not sure how to
  do it; the printed PID should reflect the current PID namespace
  and I don't how how to get this information).

* or it has been decided on purpose (i.e. it's not a bug);
  /proc/$PID/sched then offers a reliable way to get the real PID
  of any process, including processes that run in a child PID
  namespace.

* or it's a bug but it cannot be corrected (kernel ABI ; it has been
  here for a looooong time - possibly since the PID namespace
  integration).

(There might be other options here).

In the two last cases there is no need for a patch but I'd be happy
if someone explains the reasoning.

Best regards,

-- Emmanuel Deloget

[1]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/kernel/sched/debug.c?id=refs/tags/v3.11#n495
[2]: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/kernel/sched/debug.c?id=refs/tags/v3.11#n118



[-- Attachment #2: emmanuel_deloget.vcf --]
[-- Type: text/x-vcard, Size: 280 bytes --]

begin:vcard
fn:Emmanuel Deloget
n:Deloget;Emmanuel
org:efixo / SFR;DATA
adr;quoted-printable:;;67 mont=C3=A9e de St Menet;MARSEILLE;;13011;FRANCE
email;internet:emmanuel.deloget@efixo.com
title:Team Leader
tel;work:04 88 15 50 77
url:www.sfr.fr
version:2.1
end:vcard


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: /proc/$PID/sched does not take PID namespace into account
  2013-09-03 14:22 /proc/$PID/sched does not take PID namespace into account Emmanuel Deloget
@ 2013-09-09 11:01 ` Peter Zijlstra
  2013-09-09 13:58   ` Emmanuel Deloget
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Peter Zijlstra @ 2013-09-09 11:01 UTC (permalink / raw)
  To: Emmanuel Deloget; +Cc: [ML] linux-kernel, Ingo Molnar

On Tue, Sep 03, 2013 at 04:22:33PM +0200, Emmanuel Deloget wrote:
> The issue (if this is really an issue) lies in kernel/sched/debug.c,
> function proc_sched_show_task(). The code says [1]:
> 
>     SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
>                         get_nr_threads(p));
> 

> I see a few options:
> 
> * either it's a bug and it should be corrected (I'm not sure how to
>   do it; the printed PID should reflect the current PID namespace
>   and I don't how how to get this information).

I suppose something like the below ought to work? -- completely
untested, please confirm.

---
Subject: sched, debug: Use PID namespaces

Emmanuel reported that /proc/sched_debug didn't report the right PIDs
when using namespaces, cure this.

Reported-by: Emmanuel Deloget <emmanuel.deloget@efixo.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
---
 kernel/sched/debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index e076bdd..e30bf44 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -124,7 +124,7 @@ print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
 		SEQ_printf(m, " ");
 
 	SEQ_printf(m, "%15s %5d %9Ld.%06ld %9Ld %5d ",
-		p->comm, p->pid,
+		p->comm, task_pid(p),,
 		SPLIT_NS(p->se.vruntime),
 		(long long)(p->nvcsw + p->nivcsw),
 		p->prio);
@@ -289,7 +289,7 @@ do {									\
 	P(nr_load_updates);
 	P(nr_uninterruptible);
 	PN(next_balance);
-	P(curr->pid);
+	P(task_pid(curr));
 	PN(clock);
 	P(cpu_load[0]);
 	P(cpu_load[1]);
@@ -492,7 +492,7 @@ void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
 {
 	unsigned long nr_switches;
 
-	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
+	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid(p),
 						get_nr_threads(p));
 	SEQ_printf(m,
 		"---------------------------------------------------------"

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: /proc/$PID/sched does not take PID namespace into account
  2013-09-09 11:01 ` Peter Zijlstra
@ 2013-09-09 13:58   ` Emmanuel Deloget
  2013-09-10  9:18   ` Peter Zijlstra
  2013-09-12 18:05   ` [tip:sched/core] sched/debug: Take " tip-bot for Peter Zijlstra
  2 siblings, 0 replies; 5+ messages in thread
From: Emmanuel Deloget @ 2013-09-09 13:58 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: [ML] linux-kernel, Ingo Molnar

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

Hello,

On 09/09/2013 13:01, Peter Zijlstra wrote:
> On Tue, Sep 03, 2013 at 04:22:33PM +0200, Emmanuel Deloget wrote:
>> The issue (if this is really an issue) lies in kernel/sched/debug.c,
>> function proc_sched_show_task(). The code says [1]:
>>
>>     SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
>>                         get_nr_threads(p));
>>
>> I see a few options:
>>
>> * either it's a bug and it should be corrected (I'm not sure how to
>>   do it; the printed PID should reflect the current PID namespace
>>   and I don't how how to get this information).
> I suppose something like the below ought to work? -- completely
> untested, please confirm.

Thanks !

I'll test that as soon as possible and I'll report to you.

Best regards,

-- Emmanuel Deloget

>
> ---
> Subject: sched, debug: Use PID namespaces
>
> Emmanuel reported that /proc/sched_debug didn't report the right PIDs
> when using namespaces, cure this.
>
> Reported-by: Emmanuel Deloget <emmanuel.deloget@efixo.com>
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> ---
>  kernel/sched/debug.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index e076bdd..e30bf44 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -124,7 +124,7 @@ print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
>  		SEQ_printf(m, " ");
>  
>  	SEQ_printf(m, "%15s %5d %9Ld.%06ld %9Ld %5d ",
> -		p->comm, p->pid,
> +		p->comm, task_pid(p),,
>  		SPLIT_NS(p->se.vruntime),
>  		(long long)(p->nvcsw + p->nivcsw),
>  		p->prio);
> @@ -289,7 +289,7 @@ do {									\
>  	P(nr_load_updates);
>  	P(nr_uninterruptible);
>  	PN(next_balance);
> -	P(curr->pid);
> +	P(task_pid(curr));
>  	PN(clock);
>  	P(cpu_load[0]);
>  	P(cpu_load[1]);
> @@ -492,7 +492,7 @@ void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
>  {
>  	unsigned long nr_switches;
>  
> -	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
> +	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid(p),
>  						get_nr_threads(p));
>  	SEQ_printf(m,
>  		"---------------------------------------------------------"
> .
>


-- 

[-- Attachment #2: emmanuel_deloget.vcf --]
[-- Type: text/x-vcard, Size: 268 bytes --]

begin:vcard
fn:Emmanuel Deloget
n:Deloget;Emmanuel
org:efixo / SFR;DATA
adr;quoted-printable:;;67 mont=C3=A9e de St Menet;MARSEILLE;;13011;FRANCE
email;internet:emmanuel.deloget@efixo.com
title:Team Leader
tel;work:04 88 15 50 77
url:www.sfr.fr
version:2.1
end:vcard


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: /proc/$PID/sched does not take PID namespace into account
  2013-09-09 11:01 ` Peter Zijlstra
  2013-09-09 13:58   ` Emmanuel Deloget
@ 2013-09-10  9:18   ` Peter Zijlstra
  2013-09-12 18:05   ` [tip:sched/core] sched/debug: Take " tip-bot for Peter Zijlstra
  2 siblings, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2013-09-10  9:18 UTC (permalink / raw)
  To: Emmanuel Deloget; +Cc: [ML] linux-kernel, Ingo Molnar

On Mon, Sep 09, 2013 at 01:01:41PM +0200, Peter Zijlstra wrote:
> On Tue, Sep 03, 2013 at 04:22:33PM +0200, Emmanuel Deloget wrote:
> > The issue (if this is really an issue) lies in kernel/sched/debug.c,
> > function proc_sched_show_task(). The code says [1]:
> > 
> >     SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
> >                         get_nr_threads(p));
> > 
> 
> > I see a few options:
> > 
> > * either it's a bug and it should be corrected (I'm not sure how to
> >   do it; the printed PID should reflect the current PID namespace
> >   and I don't how how to get this information).
> 
> I suppose something like the below ought to work? -- completely
> untested, please confirm.
> 
> ---
> Subject: sched, debug: Use PID namespaces
> 
> Emmanuel reported that /proc/sched_debug didn't report the right PIDs
> when using namespaces, cure this.
> 
> Reported-by: Emmanuel Deloget <emmanuel.deloget@efixo.com>
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> ---
>  kernel/sched/debug.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index e076bdd..e30bf44 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -124,7 +124,7 @@ print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
>  		SEQ_printf(m, " ");
>  
>  	SEQ_printf(m, "%15s %5d %9Ld.%06ld %9Ld %5d ",
> -		p->comm, p->pid,
> +		p->comm, task_pid(p),,
>  		SPLIT_NS(p->se.vruntime),
>  		(long long)(p->nvcsw + p->nivcsw),
>  		p->prio);
> @@ -289,7 +289,7 @@ do {									\
>  	P(nr_load_updates);
>  	P(nr_uninterruptible);
>  	PN(next_balance);
> -	P(curr->pid);
> +	P(task_pid(curr));
>  	PN(clock);
>  	P(cpu_load[0]);
>  	P(cpu_load[1]);
> @@ -492,7 +492,7 @@ void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
>  {
>  	unsigned long nr_switches;
>  
> -	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
> +	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid(p),
>  						get_nr_threads(p));
>  	SEQ_printf(m,
>  		"---------------------------------------------------------"

Please do s/task_pid/task_pid_nr/g on this before trying. 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip:sched/core] sched/debug: Take PID namespace into account
  2013-09-09 11:01 ` Peter Zijlstra
  2013-09-09 13:58   ` Emmanuel Deloget
  2013-09-10  9:18   ` Peter Zijlstra
@ 2013-09-12 18:05   ` tip-bot for Peter Zijlstra
  2 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Peter Zijlstra @ 2013-09-12 18:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, peterz, tglx, emmanuel.deloget

Commit-ID:  fc840914e9b07ab4685c195e1e54e58de4f84c03
Gitweb:     http://git.kernel.org/tip/fc840914e9b07ab4685c195e1e54e58de4f84c03
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Mon, 9 Sep 2013 13:01:41 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 12 Sep 2013 19:14:16 +0200

sched/debug: Take PID namespace into account

Emmanuel reported that /proc/sched_debug didn't report the right PIDs
when using namespaces, cure this.

Reported-by: Emmanuel Deloget <emmanuel.deloget@efixo.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20130909110141.GM31370@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index e076bdd..1965599 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -124,7 +124,7 @@ print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
 		SEQ_printf(m, " ");
 
 	SEQ_printf(m, "%15s %5d %9Ld.%06ld %9Ld %5d ",
-		p->comm, p->pid,
+		p->comm, task_pid_nr(p),
 		SPLIT_NS(p->se.vruntime),
 		(long long)(p->nvcsw + p->nivcsw),
 		p->prio);
@@ -289,7 +289,7 @@ do {									\
 	P(nr_load_updates);
 	P(nr_uninterruptible);
 	PN(next_balance);
-	P(curr->pid);
+	SEQ_printf(m, "  .%-30s: %ld\n", "curr->pid", (long)(task_pid_nr(rq->curr)));
 	PN(clock);
 	P(cpu_load[0]);
 	P(cpu_load[1]);
@@ -492,7 +492,7 @@ void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
 {
 	unsigned long nr_switches;
 
-	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, p->pid,
+	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_nr(p),
 						get_nr_threads(p));
 	SEQ_printf(m,
 		"---------------------------------------------------------"

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-09-12 18:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-03 14:22 /proc/$PID/sched does not take PID namespace into account Emmanuel Deloget
2013-09-09 11:01 ` Peter Zijlstra
2013-09-09 13:58   ` Emmanuel Deloget
2013-09-10  9:18   ` Peter Zijlstra
2013-09-12 18:05   ` [tip:sched/core] sched/debug: Take " tip-bot for Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox