fs, proc: Introduce /proc/<pid>/task/<tid>/children entry v5
Subsystems (2)
RFC 1/1 fs, proc: Introduce /proc/<pid>/task/<tid>/children entry v5
When we do checkpoint of a task we need to know the list of children the task has but there is no easy way to make a reverse parent->children chain from an arbitrary <pid> (while a parent pid is provided in "PPid" field of /proc/<pid>/status).
So instead of walking over all pids in the system creating the big process tree just to figure out which children a task has -- we add the explicit /proc/<pid>/task/<tid>/children entry, because the kernel already has this kind of information but it is not yet exported.
This is a first level children, not the whole process tree.
v2:
- Kame suggested to use a separated /proc/<pid>/children entry instead of poking /proc/<pid>/status
- Andew suggested to use rcu facility instead of locking tasklist_lock
- Tejun pointed that non-seekable seq file might not be enough for tasks with large number of children
v3:
- To be on a safe side use %lu format for pid_t printing
v4:
- New line get printed when sequence ends not at seq->stop, a nit pointed by Tejun
- Documentation update
- tasklist_lock is back, Oleg pointed that ->children list is actually not rcu-safe
v5:
- Oleg suggested to make /proc/<pid>/task/<tid>/children instead of global /proc/<pid>/children, which eliminates hardness related to threads and children migration, and allows patch to be a way simplier.
--- Documentation/filesystems/proc.txt | 20 ++++ fs/proc/array.c | 162 +++++++++++++++++++++++++++++++++++++ fs/proc/base.c | 1 fs/proc/internal.h | 1 4 files changed, 183 insertions(+), 1 deletion(-)
Documentation/filesystems/proc.txt +19 -1
@@ -40,7 +40,7 @@ Table of Contents 3.4 /proc/<pid>/coredump_filter - Core dump filtering settings 3.5 /proc/<pid>/mountinfo - Information about mounts 3.6 /proc/<pid>/comm & /proc/<pid>/task/<tid>/comm - + 3.7 /proc/<pid>/task/<tid>/children - Information about task children ------------------------------------------------------------------------------ Preface
@@ -1545,3 +1545,21 @@ a task to set its own or one of its thre is limited in size compared to the cmdline value, so writing anything longer then the kernel's TASK_COMM_LEN (currently 16 chars) will result in a truncated comm value. + +3.7 /proc/<pid>/task/<tid>/children - Information about task children +------------------------------------------------------------------------- +This file provides a fast way to retrieve first level children pids +of a task pointed by <pid>/<tid> pair. The format is a space separated +stream of pids. + +Note the "first level" here -- if a child has own children they will +not be shown here, one needs to read /proc/<children-pid>/task/<tid>/children +to obtain descendants. + +Because this interface is intended to be fast and cheap it doesn't +guarantee to provide precise results, which means if a child is exiting +it might or might not be counted. The same applies to freshly created +children -- they might or might not be counted. + +If one needs precise pids -- the task and children should be stopped +or frozen.
fs/proc/array.c +162 -0
165 lines not on this page · raw message
fs/proc/base.c +1 -0
@@ -3497,6 +3497,7 @@ static const struct pid_entry tid_base_sONE ("stat" ,S_IRUGO , proc_tid_stat),ONE ("statm" ,S_IRUGO , proc_pid_statm),REG ("maps" ,S_IRUGO , proc_maps_operations), +REG ("children" ,S_IRUGO , proc_tid_children_operations), #ifdef CONFIG_NUMA REG("numa_maps", S_IRUGO, proc_numa_maps_operations), #endif
fs/proc/internal.h +1 -0
@@ -53,6 +53,7 @@ extern int proc_pid_statm(struct seq_fil struct pid *pid, struct task_struct *task);extern loff_t mem_lseek (struct file *file,loff_t offset,int orig); +extern const struct file_operations proc_tid_children_operations;extern const struct file_operations proc_maps_operations;extern const struct file_operations proc_numa_maps_operations;extern const struct file_operations proc_smaps_operations;
Quoted 5 lines · Cyrill Gorcunov
When we do checkpoint of a task we need to know the list of children the task has but there is no easy way to make a reverse
parent->childrenchain from an arbitrary <pid> (while a parent pid is provided in "PPid" field of /proc/<pid>/status).
Looks correct at first glance... But I'll try to recheck. I guess you need to resend anyway, I bet nobody can recall this patch ;)
However I do not understand the ptrace_may_access() check at all.
Quoted 20 lines
fs/proc/array.c +20
-0+static struct pid * +get_children_pid (struct proc_pid_children_iter *iter,struct pid *pid_prev,loff_t pos) +{ +struct task_struct *start, *task; +struct pid *pid =NULL ; + +read_lock (&tasklist_lock); + + start =pid_task (iter->pid_start,PIDTYPE_PID ); +if (!start) + goto out; + +/* +* Lets try to continue searching this would speed +* search significantly. +*/ +if (pid_prev) { + task =pid_task (pid_prev,PIDTYPE_PID ); +if (task && task->real_parent == start && + !(list_empty (&task->sibling))) {
fs/proc/array.c int proc_pid_statm(struct seq_file *m, s · line 585
Quoted 10 lines
fs/proc/array.c +10
-0+/* +* OK, ltes try the fastpath, we might +* miss some freshly created children +* here, but it was never promised to be +* accurate. +* +* Also note if we have not enough rights +* to access the next children pid we simply +* fall into slow-search version. +*/
Why we should try the slow-search path if ptrace_may_access() fails?
fs/proc/array.c int proc_pid_statm(struct seq_file *m, s · line 596
Quoted 11 lines
fs/proc/array.c +11
-0+if (!list_is_last (&task->sibling, &start->children)) { + task =list_first_entry (&task->sibling, + struct task_struct, sibling); +if (ptrace_may_access (task,PTRACE_MODE_READ )) { + pid =get_pid (task_pid (task)); + goto out; + } + }else + goto out; + } + }
Well, this is cosmetic, but imho
if (list_is_last(...)) goto out; task = list_first_entry(...); ...
looks better.
fs/proc/array.c int proc_pid_statm(struct seq_file *m, s · line 605
Quoted 8 lines
fs/proc/array.c +8
-0+list_for_each_entry (task, &start->children, sibling) { +if (pos-- ==0 ) { +if (ptrace_may_access (task,PTRACE_MODE_READ )) { + pid =get_pid (task_pid (task)); + goto out; + }else { +/* Maybe we success with the next children */ + pos++;
Again. I simply can't understand what ptrace_may_access() actually means. Why do we use the possible child, not parent?
IOW. I have no idea if we really need any security check at all. You can find the children pids without this patch anyway via. grep PPid /proc/*/status.
But if you want ptrace_may_access/whatever, you should check ptrace_may_access(start), no?
Oleg.
Quoted 10 lines · Oleg Nesterov
Quoted 5 lines · Cyrill Gorcunov
When we do checkpoint of a task we need to know the list of children the task has but there is no easy way to make a reverse
parent->childrenchain from an arbitrary <pid> (while a parent pid is provided in "PPid" field of /proc/<pid>/status).Looks correct at first glance... But I'll try to recheck. I guess you need to resend anyway, I bet nobody can recall this patch ;)
Sure ;)
Quoted 2 lines
However I do not understand the
ptrace_may_access()check at all.
...
Quoted 10 lines
Well, this is cosmetic, but imho
if (list_is_last(...)) goto out; task = list_first_entry(...); ...looks better.
ok
Quoted 13 lines
fs/proc/array.c int proc_pid_statm(struct seq_file *m, s · line 605
Quoted 9 lines
fs/proc/array.c +8
-0+list_for_each_entry (task, &start->children, sibling) { +if (pos-- ==0 ) { +if (ptrace_may_access (task,PTRACE_MODE_READ )) { + pid =get_pid (task_pid (task)); + goto out; + }else { +/* Maybe we success with the next children */ + pos++;Again. I simply can't understand what
ptrace_may_access()actually means. Why do we use the possible child, not parent?
Quoted 4 lines
IOW. I have no idea if we really need any security check at all. You can find the children pids without this patch anyway via. grep PPid
/proc/*/status.
OK, I see. I am actually not sure which behaviour should be there. What should we do if say we have a task with a number of children, which changed permissions of own and some of children. Look what I mean.
We have say tid A, which has children B C D, and when we read /proc/pid/task/tid/children we should see "B C D" here. But what if say A started with roots rights, then changed own permission so everyone could read this /proc/pid/task/<A>/children, but left C with root permissions only. So should we list C here? Or such scenario is impossible at all?
Quoted 3 lines
But if you want ptrace_may_access/whatever, you should check
ptrace_may_access(start), no?
Cyrill
Quoted 15 lines · Cyrill Gorcunov
Quoted 8 lines · Oleg Nesterov
Again. I simply can't understand what
ptrace_may_access()actually means. Why do we use the possible child, not parent?IOW. I have no idea if we really need any security check at all. You can find the children pids without this patch anyway via. grep PPid
/proc/*/status.OK, I see. I am actually not sure which behaviour should be there. What should we do if say we have a task with a number of children, which changed permissions of own and some of children. Look what I mean.
Quoted 5 lines
We have say tid A, which has children B C D, and when we read
/proc/pid/task/tid/childrenwe should see "B C D" here. But what if say A started with roots rights, then changed own permission so everyone could read this /proc/pid/task/<A>/children, but left C with root permissions only. So should we list C here?
Why not? What is the problem to know the pid of this child? And once again, you can list all children without this patch anyway.
Perhaps I missed something, but I simply don't understand the problem.
Oleg.
Quoted 6 lines · Oleg Nesterov
Why not? What is the problem to know the pid of this child? And once again, you can list all children without this patch anyway.
Perhaps I missed something, but I simply don't understand the problem.
Dunno, that's why I asked. But indeed I somehow forgot that I can find all children anyway. I'll drop this snippets. Thanks!
Cyrill