mirror of
https://github.com/torvalds/linux.git
synced 2026-11-09 05:34:02 +01:00
pidfs: don't let PIDFD_GET_INFO miss a reaped task
PIDFD_GET_INFO describes the task while there is one, and with
PIDFD_INFO_EXIT its exit status once it has been reaped. pidfd_info()
looks at the two in the wrong order and can miss both:
pidfd_info() release_task()
PIDFS_ATTR_BIT_EXIT is not set
pidfs_exit()
__unhash_process()
get_pid_task() == NULL
goto copy_out
The ioctl succeeds with neither PIDFD_INFO_PID nor PIDFD_INFO_EXIT in the
mask. If the task is unhashed a little later, after get_pid_task(), its
pid reads as 0 and the ioctl fails with -ESRCH, although the caller asked
for the exit information and it is available by now.
The second case does not even need an exit. When a sub-thread execs,
de_thread() hands the leader's pid over to it and releases the old leader.
A caller that got hold of the old leader reports the tid of the sub-thread
as the pid, or -ESRCH for a process that is alive.
Look up the task first. pidfs_exit() runs before the task is unhashed, so
a caller that finds no task finds the exit information; this needs a
barrier on both sides because taking tasklist_lock does not order the
store in pidfs_exit() against the one in detach_pid(). And start over if
the task no longer owns the pid when all the fields have been read: then
there is either exit information or another task.
Go by the pid for that check and for the pid we report, not by
->thread_pid of the task. exchange_tids() moves the pid to the exec'ing
thread before it updates ->thread_pid of either task. A caller that
finds the new leader in between would read the old tid of the sub-thread
from ->thread_pid, and a check of ->thread_pid would restart until the
exec'ing thread gets to its store. On PREEMPT_RT that thread can be
preempted with tasklist_lock held and can't move to another CPU. If the
RT task that preempted it calls PIDFD_GET_INFO, the ioctl spins until
the exec'ing thread gets the CPU back, and that never happens if the
exec'ing thread is an RT task of lower priority.
Christian Brauner <brauner@kernel.org> says:
Go by @pid instead of ->thread_pid of the task for the reported pid and
for the restart check.
Fixes: 7477d7dce4 ("pidfs: allow to retrieve exit information")
Fixes: cdda1f26e7 ("pidfd: add ioctl to retrieve pid info")
Cc: stable@vger.kernel.org
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
This commit is contained in:
parent
da29ac46d3
commit
172658cf60
43
fs/pidfs.c
43
fs/pidfs.c
|
|
@ -362,6 +362,12 @@ static __u32 pidfs_coredump_mask(enum task_dumpable dumpable)
|
|||
PIDFD_INFO_COREDUMP_SIGNAL | \
|
||||
PIDFD_INFO_COREDUMP_CODE)
|
||||
|
||||
static bool pidfs_task_owns_pid(struct task_struct *task, struct pid *pid)
|
||||
{
|
||||
guard(rcu)();
|
||||
return pid_task(pid, PIDTYPE_PID) == task;
|
||||
}
|
||||
|
||||
static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
|
||||
{
|
||||
struct pidfd_info __user *uinfo = (struct pidfd_info __user *)arg;
|
||||
|
|
@ -392,6 +398,16 @@ static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
|
|||
return -EREMOTE;
|
||||
|
||||
attr = READ_ONCE(pid->attr);
|
||||
again:
|
||||
/*
|
||||
* Look for the task first. pidfs_exit() records the exit information
|
||||
* before the task is unhashed, so if the task is gone the information
|
||||
* is there.
|
||||
*/
|
||||
task = get_pid_task(pid, PIDTYPE_PID);
|
||||
if (!task)
|
||||
smp_rmb(); /* pairs with smp_mb__after_atomic() in pidfs_exit() */
|
||||
|
||||
if (mask & PIDFD_INFO_EXIT) {
|
||||
if (test_bit(PIDFS_ATTR_BIT_EXIT, &attr->attr_mask)) {
|
||||
smp_rmb();
|
||||
|
|
@ -414,7 +430,6 @@ static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
|
|||
}
|
||||
}
|
||||
|
||||
task = get_pid_task(pid, PIDTYPE_PID);
|
||||
if (!task) {
|
||||
/*
|
||||
* If the task has already been reaped, only exit
|
||||
|
|
@ -468,16 +483,27 @@ static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg)
|
|||
* task might return as soon as the copy_to_user finishes, but that's ok
|
||||
* and userspace expects that might happen and can act accordingly, so
|
||||
* this is just best-effort. What we can do however is checking that all
|
||||
* the fields are set correctly, or return ESRCH to avoid providing
|
||||
* incomplete information. */
|
||||
* the fields are set correctly, to avoid providing incomplete
|
||||
* information.
|
||||
*/
|
||||
|
||||
kinfo.ppid = task_ppid_vnr(task);
|
||||
kinfo.tgid = task_tgid_vnr(task);
|
||||
kinfo.pid = task_pid_vnr(task);
|
||||
kinfo.pid = pid_vnr(pid);
|
||||
kinfo.mask |= PIDFD_INFO_PID;
|
||||
|
||||
if (kinfo.pid == 0 || kinfo.tgid == 0)
|
||||
return -ESRCH;
|
||||
/*
|
||||
* The task was reaped, or lost @pid to a sub-thread that called exec,
|
||||
* while we were looking at it. Start over: there is exit information
|
||||
* now, or a new task. Don't go by ->thread_pid, exchange_tids()
|
||||
* updates it only after it has moved @pid.
|
||||
*/
|
||||
if (!pidfs_task_owns_pid(task, pid) || kinfo.tgid == 0) {
|
||||
put_task_struct(task);
|
||||
task = NULL;
|
||||
memset(&kinfo, 0, sizeof(kinfo));
|
||||
goto again;
|
||||
}
|
||||
|
||||
copy_out:
|
||||
if (mask & PIDFD_INFO_SUPPORTED_MASK) {
|
||||
|
|
@ -790,6 +816,11 @@ void pidfs_exit(struct task_struct *tsk)
|
|||
/* Ensure that PIDFD_GET_INFO sees either all or nothing. */
|
||||
smp_wmb();
|
||||
set_bit(PIDFS_ATTR_BIT_EXIT, &attr->attr_mask);
|
||||
/*
|
||||
* Whoever sees that the task is unhashed must see the bit, see
|
||||
* pidfd_info(). Taking tasklist_lock does not order the two stores.
|
||||
*/
|
||||
smp_mb__after_atomic();
|
||||
}
|
||||
|
||||
#ifdef CONFIG_COREDUMP
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user