Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

pidfd: check pid has attached task in fdinfo

Currently, when a task is dead we still print the pid it used to use in
the fdinfo files of its pidfds. This doesn't make much sense since the
pid may have already been reused. So verify that the task is still alive
by introducing the pid_has_task() helper which will be used by other
callers in follow-up patches.
If the task is not alive anymore, we will print -1. This allows us to
differentiate between a task not being present in a given pid namespace
- in which case we already print 0 - and a task having been reaped.

Note that this uses PIDTYPE_PID for the check. Technically, we could've
checked PIDTYPE_TGID since pidfds currently only refer to thread-group
leaders but if they won't anymore in the future then this check becomes
problematic without it being immediately obvious to non-experts imho. If
a thread is created via clone(CLONE_THREAD) than struct pid has a single
non-empty list pid->tasks[PIDTYPE_PID] and this pid can't be used as a
PIDTYPE_TGID meaning pid->tasks[PIDTYPE_TGID] will return NULL even
though the thread-group leader might still be very much alive. So
checking PIDTYPE_PID is fine and is easier to maintain should we ever
allow pidfds to refer to threads.

Cc: Jann Horn <jannh@google.com>
Cc: Christian Kellner <christian@kellner.me>
Cc: linux-api@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20191017101832.5985-1-christian.brauner@ubuntu.com

+15 -6
+4
include/linux/pid.h
··· 85 85 86 86 extern void put_pid(struct pid *pid); 87 87 extern struct task_struct *pid_task(struct pid *pid, enum pid_type); 88 + static inline bool pid_has_task(struct pid *pid, enum pid_type type) 89 + { 90 + return !hlist_empty(&pid->tasks[type]); 91 + } 88 92 extern struct task_struct *get_pid_task(struct pid *pid, enum pid_type); 89 93 90 94 extern struct pid *get_task_pid(struct task_struct *task, enum pid_type type);
+11 -6
kernel/fork.c
··· 1732 1732 */ 1733 1733 static void pidfd_show_fdinfo(struct seq_file *m, struct file *f) 1734 1734 { 1735 - struct pid_namespace *ns = proc_pid_ns(file_inode(m->file)); 1736 1735 struct pid *pid = f->private_data; 1737 - pid_t nr = pid_nr_ns(pid, ns); 1736 + struct pid_namespace *ns; 1737 + pid_t nr = -1; 1738 1738 1739 - seq_put_decimal_ull(m, "Pid:\t", nr); 1739 + if (likely(pid_has_task(pid, PIDTYPE_PID))) { 1740 + ns = proc_pid_ns(file_inode(m->file)); 1741 + nr = pid_nr_ns(pid, ns); 1742 + } 1743 + 1744 + seq_put_decimal_ll(m, "Pid:\t", nr); 1740 1745 1741 1746 #ifdef CONFIG_PID_NS 1742 - seq_put_decimal_ull(m, "\nNSpid:\t", nr); 1743 - if (nr) { 1747 + seq_put_decimal_ll(m, "\nNSpid:\t", nr); 1748 + if (nr > 0) { 1744 1749 int i; 1745 1750 1746 1751 /* If nr is non-zero it means that 'pid' is valid and that ··· 1754 1749 * Start at one below the already printed level. 1755 1750 */ 1756 1751 for (i = ns->level + 1; i <= pid->level; i++) 1757 - seq_put_decimal_ull(m, "\t", pid->numbers[i].nr); 1752 + seq_put_decimal_ll(m, "\t", pid->numbers[i].nr); 1758 1753 } 1759 1754 #endif 1760 1755 seq_putc(m, '\n');