I am dropping this one for now, as there are no dependencies with the other patches, and this probably deserves some discussion on it's own.
Eric "Eric W. Biederman" <[email protected]> writes: > Since PTRACE_KILL was introduced in 1.1.78 it has only worked if the > process is stopped in do_signal. On a ptraced but non-stopped process > PTRACE_KILL has always returned success and done nothing. > > Separate the noop case of PTRACE_KILL from the case where it does > nothing. This fixes the fact that taking sighand lock in > ptrace_resume is not safe if the process could be in the middle of > exec or do_exit. The current test for child->state is insufficient to > prevent that race. > > With the code explicitly implementing the noop people maintaining > ptrace no longer need to worry what happens in PTRACE_KILL if the > process is not stopped. > > The alternative fix is to change the implementation of PTRACE_KILL > to just be send_sig(SIGKILL, child, 1); But I don't know if anything > depends on the current documented behavior. > > Cc: Oleg Nesterov <[email protected]> > Cc: [email protected] > Fixes: b72c186999e6 ("ptrace: fix race between ptrace_resume() and > wait_task_stopped()") > Signed-off-by: "Eric W. Biederman" <[email protected]> > --- > kernel/ptrace.c | 43 ++++++++++++++++++++++++++----------------- > 1 file changed, 26 insertions(+), 17 deletions(-) > > diff --git a/kernel/ptrace.c b/kernel/ptrace.c > index 6f357f4fc859..5d6ff7040863 100644 > --- a/kernel/ptrace.c > +++ b/kernel/ptrace.c > @@ -212,15 +212,18 @@ static void ptrace_unfreeze_traced(struct task_struct > *task) > * > * Check whether @child is being ptraced by %current and ready for further > * ptrace operations. If @ignore_state is %false, @child also should be in > - * %TASK_TRACED state and on return the child is guaranteed to be traced > - * and not executing. If @ignore_state is %true, @child can be in any > - * state. > + * %TASK_TRACED state and on succesful return the child is guaranteed to be > + * traced and not executing. If @ignore_state is %true, @child can be in > + * any state on succesful return. > * > * CONTEXT: > * Grabs and releases tasklist_lock and @child->sighand->siglock. > * > * RETURNS: > - * 0 on success, -ESRCH if %child is not ready. > + * 0 on success, > + * -ESRCH if %child is not traced > + * -EAGAIN if %child can not be frozen > + * -EBUSY if the wait for %child fails > */ > static int ptrace_check_attach(struct task_struct *child, bool ignore_state) > { > @@ -240,6 +243,7 @@ static int ptrace_check_attach(struct task_struct *child, > bool ignore_state) > * child->sighand can't be NULL, release_task() > * does ptrace_unlink() before __exit_signal(). > */ > + ret = -EAGAIN; > if (ignore_state || ptrace_freeze_traced(child)) > ret = 0; > } > @@ -253,7 +257,7 @@ static int ptrace_check_attach(struct task_struct *child, > bool ignore_state) > * so we should not worry about leaking __TASK_TRACED. > */ > WARN_ON(child->state == __TASK_TRACED); > - ret = -ESRCH; > + ret = -EBUSY; > } > } > > @@ -1074,8 +1078,6 @@ int ptrace_request(struct task_struct *child, long > request, > return ptrace_resume(child, request, data); > > case PTRACE_KILL: > - if (child->exit_state) /* already dead */ > - return 0; > return ptrace_resume(child, request, SIGKILL); > > #ifdef CONFIG_HAVE_ARCH_TRACEHOOK > @@ -1147,14 +1149,17 @@ SYSCALL_DEFINE4(ptrace, long, request, long, pid, > unsigned long, addr, > goto out_put_task_struct; > } > > - ret = ptrace_check_attach(child, request == PTRACE_KILL || > - request == PTRACE_INTERRUPT); > - if (ret < 0) > - goto out_put_task_struct; > - > - ret = arch_ptrace(child, request, addr, data); > - if (ret || request != PTRACE_DETACH) > - ptrace_unfreeze_traced(child); > + ret = ptrace_check_attach(child, request == PTRACE_INTERRUPT); > + if (!ret) { > + ret = arch_ptrace(child, request, addr, data); > + if (ret || request != PTRACE_DETACH) > + ptrace_unfreeze_traced(child); > + } > + /* PTRACE_KILL is a noop when not attached */ > + else if ((request == PTRACE_KILL) && (ret != -ESRCH)) > + ret = 0; > + else > + ret = -ESRCH; > > out_put_task_struct: > put_task_struct(child); > @@ -1292,13 +1297,17 @@ COMPAT_SYSCALL_DEFINE4(ptrace, compat_long_t, > request, compat_long_t, pid, > goto out_put_task_struct; > } > > - ret = ptrace_check_attach(child, request == PTRACE_KILL || > - request == PTRACE_INTERRUPT); > + ret = ptrace_check_attach(child, request == PTRACE_INTERRUPT); > if (!ret) { > ret = compat_arch_ptrace(child, request, addr, data); > if (ret || request != PTRACE_DETACH) > ptrace_unfreeze_traced(child); > } > + /* PTRACE_KILL is a noop when not attached */ > + else if ((request == PTRACE_KILL) && (ret != -ESRCH)) > + ret = 0; > + else > + ret = -ESRCH; > > out_put_task_struct: > put_task_struct(child);
