On 10/29, Oleg Nesterov wrote:
>
> On 10/27, Roland McGrath wrote:
> >
> > > + case PTRACE_KILL:
> > > + ret = 0;
> > > + if (!child->exit_state) /* already dead */
> > > + ret = ptrace_resume(child, engine, request, SIGKILL);
> > > + break;
> >
> > This could be inside ptrace_resume too.
>
> Yes, and we don't even need to check exit_state.
Damn, this PTRACE_KILL is so weird.
If we move this "case:" into ptrace_resume() path, then we have to
make sure we do not return -ESRCH if request == SIGKILL (or we can
ignore this incompatibility?)
Apart from that, what it should actually do?
If the tracee was STOPPED/TRACED, then it is equivalent to PTRACE_CONT,SIGKILL.
Otherwise it has no effect except it can race with exit() and spoil ->exit_code,
and wake_up_process() is not good if course. (I am talking about upstream).
Perhaps it is better to leave this strange and special case in ptrace_request()
like this patch does?
Otherwise, we can change ptrace_resume() to never return the error if, say,
utrace_set_events() fails, but while this is correct it looks a bit ugly.
Please tell me what do you think, then I'll do other changes you suggested.
---
kernel/ptrace.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--- PU/kernel/ptrace.c~122_PTRACE_KILL 2009-10-26 03:18:12.000000000 +0100
+++ PU/kernel/ptrace.c 2009-10-30 04:00:15.000000000 +0100
@@ -1000,9 +1000,10 @@ int ptrace_request(struct task_struct *c
break;
case PTRACE_KILL:
+ /* Ugly historical behaviour. */
+ if (task_is_traced(child))
+ ptrace_resume(child, engine, PTRACE_CONT, SIGKILL);
ret = 0;
- if (!child->exit_state) /* already dead */
- ret = ptrace_resume(child, engine, request, SIGKILL);
break;
default:
