All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] sched: debug: use task_pid_nr_ns in /proc/$pid/sched
@ 2017-08-06  4:41 Aleksa Sarai
  2017-08-06 14:56 ` Eric W. Biederman
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Aleksa Sarai @ 2017-08-06  4:41 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Eric W. Biederman
  Cc: cyphar, linux-kernel, Aleksa Sarai, stable, Jess Frazelle

It appears as though the addition of the PID namespace did not update
the output code for /proc/*/sched, which resulted in it providing PIDs
that were not self-consistent with the /proc mount. This additionally
made it trivial to detect whether a process was inside &init_pid_ns from
userspace (making container detection trivial[1]). This lead to
situations such as:

  % unshare -pmf
  % mount -t proc proc /proc
  % head -n1 /proc/1/sched
  head (10047, #threads: 1)

Fix this by just using task_pid_nr_ns for the output of /proc/*/sched.
All of the other uses of task_pid_nr in kernel/sched/debug.c are from a
sysctl context and thus don't need to be namespaced.

[1]: https://github.com/jessfraz/amicontained

Cc: <stable@vger.kernel.org>
Cc: Jess Frazelle <acidburn@google.com>
Signed-off-by: Aleksa Sarai <asarai@suse.com>
---
 fs/proc/base.c              | 3 ++-
 include/linux/sched/debug.h | 4 +++-
 kernel/sched/debug.c        | 5 +++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 719c2e943ea1..98fd8f6df851 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1408,12 +1408,13 @@ static const struct file_operations proc_fail_nth_operations = {
 static int sched_show(struct seq_file *m, void *v)
 {
 	struct inode *inode = m->private;
+	struct pid_namespace *ns = inode->i_sb->s_fs_info;
 	struct task_struct *p;
 
 	p = get_proc_task(inode);
 	if (!p)
 		return -ESRCH;
-	proc_sched_show_task(p, m);
+	proc_sched_show_task(p, ns, m);
 
 	put_task_struct(p);
 
diff --git a/include/linux/sched/debug.h b/include/linux/sched/debug.h
index e0eaee54c5a4..5d58d49e9f87 100644
--- a/include/linux/sched/debug.h
+++ b/include/linux/sched/debug.h
@@ -6,6 +6,7 @@
  */
 
 struct task_struct;
+struct pid_namespace;
 
 extern void dump_cpu_task(int cpu);
 
@@ -34,7 +35,8 @@ extern void sched_show_task(struct task_struct *p);
 
 #ifdef CONFIG_SCHED_DEBUG
 struct seq_file;
-extern void proc_sched_show_task(struct task_struct *p, struct seq_file *m);
+extern void proc_sched_show_task(struct task_struct *p,
+				 struct pid_namespace *ns, struct seq_file *m);
 extern void proc_sched_set_task(struct task_struct *p);
 #endif
 
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 4fa66de52bd6..ac345115877b 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -872,11 +872,12 @@ static void sched_show_numa(struct task_struct *p, struct seq_file *m)
 #endif
 }
 
-void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
+void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
+						  struct seq_file *m)
 {
 	unsigned long nr_switches;
 
-	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_nr(p),
+	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_nr_ns(p, ns),
 						get_nr_threads(p));
 	SEQ_printf(m,
 		"---------------------------------------------------------"
-- 
2.13.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] sched: debug: use task_pid_nr_ns in /proc/$pid/sched
  2017-08-06  4:41 [PATCH v2] sched: debug: use task_pid_nr_ns in /proc/$pid/sched Aleksa Sarai
@ 2017-08-06 14:56 ` Eric W. Biederman
  2017-08-07  8:44 ` Peter Zijlstra
  2017-08-10 12:09 ` [tip:sched/core] sched/debug: Use " tip-bot for Aleksa Sarai
  2 siblings, 0 replies; 6+ messages in thread
From: Eric W. Biederman @ 2017-08-06 14:56 UTC (permalink / raw)
  To: Aleksa Sarai
  Cc: Ingo Molnar, Peter Zijlstra, cyphar, linux-kernel, stable, Jess Frazelle

Aleksa Sarai <asarai@suse.com> writes:

> It appears as though the addition of the PID namespace did not update
> the output code for /proc/*/sched, which resulted in it providing PIDs
> that were not self-consistent with the /proc mount. This additionally
> made it trivial to detect whether a process was inside &init_pid_ns from
> userspace (making container detection trivial[1]). This lead to
> situations such as:
>
>   % unshare -pmf
>   % mount -t proc proc /proc
>   % head -n1 /proc/1/sched
>   head (10047, #threads: 1)
>
> Fix this by just using task_pid_nr_ns for the output of /proc/*/sched.
> All of the other uses of task_pid_nr in kernel/sched/debug.c are from a
> sysctl context and thus don't need to be namespaced.
>
> [1]: https://github.com/jessfraz/amicontained
>
> Cc: <stable@vger.kernel.org>
> Cc: Jess Frazelle <acidburn@google.com>
> Signed-off-by: Aleksa Sarai <asarai@suse.com>

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

> ---
>  fs/proc/base.c              | 3 ++-
>  include/linux/sched/debug.h | 4 +++-
>  kernel/sched/debug.c        | 5 +++--
>  3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 719c2e943ea1..98fd8f6df851 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1408,12 +1408,13 @@ static const struct file_operations proc_fail_nth_operations = {
>  static int sched_show(struct seq_file *m, void *v)
>  {
>  	struct inode *inode = m->private;
> +	struct pid_namespace *ns = inode->i_sb->s_fs_info;
>  	struct task_struct *p;
>  
>  	p = get_proc_task(inode);
>  	if (!p)
>  		return -ESRCH;
> -	proc_sched_show_task(p, m);
> +	proc_sched_show_task(p, ns, m);
>  
>  	put_task_struct(p);
>  
> diff --git a/include/linux/sched/debug.h b/include/linux/sched/debug.h
> index e0eaee54c5a4..5d58d49e9f87 100644
> --- a/include/linux/sched/debug.h
> +++ b/include/linux/sched/debug.h
> @@ -6,6 +6,7 @@
>   */
>  
>  struct task_struct;
> +struct pid_namespace;
>  
>  extern void dump_cpu_task(int cpu);
>  
> @@ -34,7 +35,8 @@ extern void sched_show_task(struct task_struct *p);
>  
>  #ifdef CONFIG_SCHED_DEBUG
>  struct seq_file;
> -extern void proc_sched_show_task(struct task_struct *p, struct seq_file *m);
> +extern void proc_sched_show_task(struct task_struct *p,
> +				 struct pid_namespace *ns, struct seq_file *m);
>  extern void proc_sched_set_task(struct task_struct *p);
>  #endif
>  
> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> index 4fa66de52bd6..ac345115877b 100644
> --- a/kernel/sched/debug.c
> +++ b/kernel/sched/debug.c
> @@ -872,11 +872,12 @@ static void sched_show_numa(struct task_struct *p, struct seq_file *m)
>  #endif
>  }
>  
> -void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
> +void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
> +						  struct seq_file *m)
>  {
>  	unsigned long nr_switches;
>  
> -	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_nr(p),
> +	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_nr_ns(p, ns),
>  						get_nr_threads(p));
>  	SEQ_printf(m,
>  		"---------------------------------------------------------"

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] sched: debug: use task_pid_nr_ns in /proc/$pid/sched
  2017-08-06  4:41 [PATCH v2] sched: debug: use task_pid_nr_ns in /proc/$pid/sched Aleksa Sarai
  2017-08-06 14:56 ` Eric W. Biederman
@ 2017-08-07  8:44 ` Peter Zijlstra
  2017-08-07 15:18   ` Jessie Frazelle
  2017-08-10 12:09 ` [tip:sched/core] sched/debug: Use " tip-bot for Aleksa Sarai
  2 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2017-08-07  8:44 UTC (permalink / raw)
  To: Aleksa Sarai
  Cc: Ingo Molnar, Eric W. Biederman, cyphar, linux-kernel, stable,
	Jess Frazelle

On Sun, Aug 06, 2017 at 02:41:41PM +1000, Aleksa Sarai wrote:
> It appears as though the addition of the PID namespace did not update
> the output code for /proc/*/sched, which resulted in it providing PIDs
> that were not self-consistent with the /proc mount. This additionally
> made it trivial to detect whether a process was inside &init_pid_ns from
> userspace (making container detection trivial[1]). This lead to
> situations such as:
> 
>   % unshare -pmf
>   % mount -t proc proc /proc
>   % head -n1 /proc/1/sched
>   head (10047, #threads: 1)
> 
> Fix this by just using task_pid_nr_ns for the output of /proc/*/sched.
> All of the other uses of task_pid_nr in kernel/sched/debug.c are from a
> sysctl context and thus don't need to be namespaced.
> 
> [1]: https://github.com/jessfraz/amicontained
> 
> Cc: <stable@vger.kernel.org>
> Cc: Jess Frazelle <acidburn@google.com>
> Signed-off-by: Aleksa Sarai <asarai@suse.com>

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] sched: debug: use task_pid_nr_ns in /proc/$pid/sched
  2017-08-07  8:44 ` Peter Zijlstra
@ 2017-08-07 15:18   ` Jessie Frazelle
  2017-08-08 15:17     ` Eric W. Biederman
  0 siblings, 1 reply; 6+ messages in thread
From: Jessie Frazelle @ 2017-08-07 15:18 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Aleksa Sarai, Ingo Molnar, Eric W. Biederman, cyphar,
	linux-kernel, stable

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

Thanks for patching this :) Guess I will find a different rabbit in a
hat to detect if running in a pid namespace ;)

On Mon, Aug 7, 2017 at 4:44 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Sun, Aug 06, 2017 at 02:41:41PM +1000, Aleksa Sarai wrote:
>> It appears as though the addition of the PID namespace did not update
>> the output code for /proc/*/sched, which resulted in it providing PIDs
>> that were not self-consistent with the /proc mount. This additionally
>> made it trivial to detect whether a process was inside &init_pid_ns from
>> userspace (making container detection trivial[1]). This lead to
>> situations such as:
>>
>>   % unshare -pmf
>>   % mount -t proc proc /proc
>>   % head -n1 /proc/1/sched
>>   head (10047, #threads: 1)
>>
>> Fix this by just using task_pid_nr_ns for the output of /proc/*/sched.
>> All of the other uses of task_pid_nr in kernel/sched/debug.c are from a
>> sysctl context and thus don't need to be namespaced.
>>
>> [1]: https://github.com/jessfraz/amicontained
>>
>> Cc: <stable@vger.kernel.org>
>> Cc: Jess Frazelle <acidburn@google.com>
>> Signed-off-by: Aleksa Sarai <asarai@suse.com>
>
> Thanks!

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4847 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] sched: debug: use task_pid_nr_ns in /proc/$pid/sched
  2017-08-07 15:18   ` Jessie Frazelle
@ 2017-08-08 15:17     ` Eric W. Biederman
  0 siblings, 0 replies; 6+ messages in thread
From: Eric W. Biederman @ 2017-08-08 15:17 UTC (permalink / raw)
  To: Jessie Frazelle
  Cc: Peter Zijlstra, Aleksa Sarai, Ingo Molnar, cyphar, linux-kernel, stable

Jessie Frazelle <jessfraz@google.com> writes:

> Thanks for patching this :) Guess I will find a different rabbit in a
> hat to detect if running in a pid namespace ;)

I want to say that stating the /proc/pid/ns/pid files ought to be an
easy way.  But thinking about it they permissions on those might be a
bit tricky.

Eric

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [tip:sched/core] sched/debug: Use task_pid_nr_ns in /proc/$pid/sched
  2017-08-06  4:41 [PATCH v2] sched: debug: use task_pid_nr_ns in /proc/$pid/sched Aleksa Sarai
  2017-08-06 14:56 ` Eric W. Biederman
  2017-08-07  8:44 ` Peter Zijlstra
@ 2017-08-10 12:09 ` tip-bot for Aleksa Sarai
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Aleksa Sarai @ 2017-08-10 12:09 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: torvalds, linux-kernel, asarai, mingo, hpa, ebiederm, acidburn,
	peterz, tglx

Commit-ID:  74dc3384fc7983b78cc46ebb1824968a3db85eb1
Gitweb:     http://git.kernel.org/tip/74dc3384fc7983b78cc46ebb1824968a3db85eb1
Author:     Aleksa Sarai <asarai@suse.com>
AuthorDate: Sun, 6 Aug 2017 14:41:41 +1000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 10 Aug 2017 12:18:19 +0200

sched/debug: Use task_pid_nr_ns in /proc/$pid/sched

It appears as though the addition of the PID namespace did not update
the output code for /proc/*/sched, which resulted in it providing PIDs
that were not self-consistent with the /proc mount. This additionally
made it trivial to detect whether a process was inside &init_pid_ns from
userspace, making container detection trivial:

   https://github.com/jessfraz/amicontained

This leads to situations such as:

  % unshare -pmf
  % mount -t proc proc /proc
  % head -n1 /proc/1/sched
  head (10047, #threads: 1)

Fix this by just using task_pid_nr_ns for the output of /proc/*/sched.
All of the other uses of task_pid_nr in kernel/sched/debug.c are from a
sysctl context and thus don't need to be namespaced.

Signed-off-by: Aleksa Sarai <asarai@suse.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Eric W. Biederman <ebiederm@xmission.com>
Cc: Jess Frazelle <acidburn@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: cyphar@cyphar.com
Link: http://lkml.kernel.org/r/20170806044141.5093-1-asarai@suse.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 fs/proc/base.c              | 3 ++-
 include/linux/sched/debug.h | 4 +++-
 kernel/sched/debug.c        | 5 +++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 719c2e9..98fd8f6 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1408,12 +1408,13 @@ static const struct file_operations proc_fail_nth_operations = {
 static int sched_show(struct seq_file *m, void *v)
 {
 	struct inode *inode = m->private;
+	struct pid_namespace *ns = inode->i_sb->s_fs_info;
 	struct task_struct *p;
 
 	p = get_proc_task(inode);
 	if (!p)
 		return -ESRCH;
-	proc_sched_show_task(p, m);
+	proc_sched_show_task(p, ns, m);
 
 	put_task_struct(p);
 
diff --git a/include/linux/sched/debug.h b/include/linux/sched/debug.h
index e0eaee5..5d58d49 100644
--- a/include/linux/sched/debug.h
+++ b/include/linux/sched/debug.h
@@ -6,6 +6,7 @@
  */
 
 struct task_struct;
+struct pid_namespace;
 
 extern void dump_cpu_task(int cpu);
 
@@ -34,7 +35,8 @@ extern void sched_show_task(struct task_struct *p);
 
 #ifdef CONFIG_SCHED_DEBUG
 struct seq_file;
-extern void proc_sched_show_task(struct task_struct *p, struct seq_file *m);
+extern void proc_sched_show_task(struct task_struct *p,
+				 struct pid_namespace *ns, struct seq_file *m);
 extern void proc_sched_set_task(struct task_struct *p);
 #endif
 
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 4fa66de..ac34511 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -872,11 +872,12 @@ static void sched_show_numa(struct task_struct *p, struct seq_file *m)
 #endif
 }
 
-void proc_sched_show_task(struct task_struct *p, struct seq_file *m)
+void proc_sched_show_task(struct task_struct *p, struct pid_namespace *ns,
+						  struct seq_file *m)
 {
 	unsigned long nr_switches;
 
-	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_nr(p),
+	SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_nr_ns(p, ns),
 						get_nr_threads(p));
 	SEQ_printf(m,
 		"---------------------------------------------------------"

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-10 12:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-06  4:41 [PATCH v2] sched: debug: use task_pid_nr_ns in /proc/$pid/sched Aleksa Sarai
2017-08-06 14:56 ` Eric W. Biederman
2017-08-07  8:44 ` Peter Zijlstra
2017-08-07 15:18   ` Jessie Frazelle
2017-08-08 15:17     ` Eric W. Biederman
2017-08-10 12:09 ` [tip:sched/core] sched/debug: Use " tip-bot for Aleksa Sarai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.