Re: [PATCH] sched: debug: use task_pid_vnr in /proc/$pid/sched
Aleksa Saraiwrites: > On 2017-08-06 02:52, Aleksa Sarai wrote: >> It appears as though the addition of the PID namespace did not update >> the output code for /proc/$pid/sched, which made it trivial to figure >> out whether a process was inside _pid_ns from userspace (making >> container detection trivial[1]). This lead to situations such as: >> >> % unshare -pf head -n1 /proc/self/sched >> head (10047, #threads: 1) >> >> Fix this by just using task_pid_vnr for the output of /proc/$pid/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: >> Cc: Jess Frazelle >> Signed-off-by: Aleksa Sarai >> --- >> kernel/sched/debug.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c >> index 4fa66de52bd6..a06acbe33e16 100644 >> --- a/kernel/sched/debug.c >> +++ b/kernel/sched/debug.c >> @@ -876,7 +876,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, task_pid_nr(p), >> +SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_vnr(p), >> get_nr_threads(p)); >> SEQ_printf(m, >> "-" > > Added Eric to Cc. Changing this to pid_nr_ns, to make this relative to the pid namespace of proc would be very reasonable. Making files content relative to current is in general a bug. Hiding the fact you are contained is not an explicit goal. But making those things that are reasonably in a namespace relative to that namespace is. Eric
Re: [PATCH] sched: debug: use task_pid_vnr in /proc/$pid/sched
Aleksa Sarai writes: > On 2017-08-06 02:52, Aleksa Sarai wrote: >> It appears as though the addition of the PID namespace did not update >> the output code for /proc/$pid/sched, which made it trivial to figure >> out whether a process was inside _pid_ns from userspace (making >> container detection trivial[1]). This lead to situations such as: >> >> % unshare -pf head -n1 /proc/self/sched >> head (10047, #threads: 1) >> >> Fix this by just using task_pid_vnr for the output of /proc/$pid/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: >> Cc: Jess Frazelle >> Signed-off-by: Aleksa Sarai >> --- >> kernel/sched/debug.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c >> index 4fa66de52bd6..a06acbe33e16 100644 >> --- a/kernel/sched/debug.c >> +++ b/kernel/sched/debug.c >> @@ -876,7 +876,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, task_pid_nr(p), >> +SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_vnr(p), >> get_nr_threads(p)); >> SEQ_printf(m, >> "-" > > Added Eric to Cc. Changing this to pid_nr_ns, to make this relative to the pid namespace of proc would be very reasonable. Making files content relative to current is in general a bug. Hiding the fact you are contained is not an explicit goal. But making those things that are reasonably in a namespace relative to that namespace is. Eric
Re: [PATCH] sched: debug: use task_pid_vnr in /proc/$pid/sched
On 2017-08-06 02:52, Aleksa Sarai wrote: It appears as though the addition of the PID namespace did not update the output code for /proc/$pid/sched, which made it trivial to figure out whether a process was inside _pid_ns from userspace (making container detection trivial[1]). This lead to situations such as: % unshare -pf head -n1 /proc/self/sched head (10047, #threads: 1) Fix this by just using task_pid_vnr for the output of /proc/$pid/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:Cc: Jess Frazelle Signed-off-by: Aleksa Sarai --- kernel/sched/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c index 4fa66de52bd6..a06acbe33e16 100644 --- a/kernel/sched/debug.c +++ b/kernel/sched/debug.c @@ -876,7 +876,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, task_pid_nr(p), + SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_vnr(p), get_nr_threads(p)); SEQ_printf(m, "-" Added Eric to Cc. -- Aleksa Sarai Software Engineer (Containers) SUSE Linux GmbH https://www.cyphar.com/
Re: [PATCH] sched: debug: use task_pid_vnr in /proc/$pid/sched
On 2017-08-06 02:52, Aleksa Sarai wrote: It appears as though the addition of the PID namespace did not update the output code for /proc/$pid/sched, which made it trivial to figure out whether a process was inside _pid_ns from userspace (making container detection trivial[1]). This lead to situations such as: % unshare -pf head -n1 /proc/self/sched head (10047, #threads: 1) Fix this by just using task_pid_vnr for the output of /proc/$pid/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: Cc: Jess Frazelle Signed-off-by: Aleksa Sarai --- kernel/sched/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c index 4fa66de52bd6..a06acbe33e16 100644 --- a/kernel/sched/debug.c +++ b/kernel/sched/debug.c @@ -876,7 +876,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, task_pid_nr(p), + SEQ_printf(m, "%s (%d, #threads: %d)\n", p->comm, task_pid_vnr(p), get_nr_threads(p)); SEQ_printf(m, "-" Added Eric to Cc. -- Aleksa Sarai Software Engineer (Containers) SUSE Linux GmbH https://www.cyphar.com/