Re: Questions about ptrace on a dying process

[Tim Bird <tim.bird@am.sony.com>]

On 02/29/2012 11:12 PM, Denys Vlasenko wrote:
> On Wednesday 29 February 2012 21:45, Tim Bird wrote:
>> On 02/29/2012 11:12 AM, Andi Kleen wrote:
>>> Tim Bird <tim.bird@am.sony.com> writes:
>>>
>>>> ptrace maintainers (and interested parties)...
>>>>
>>>> I'm working on a crash handler for Linux, which uses ptrace to retrieve information
>>>> about a process during it's coredump.  Specifically, from within a core handler
>>>> program (started within do_coredump() as a user_mode_helper), I would like to make
>>>> ptrace calls against the dying process.
>>>
>>> The standard approach is to define a core pipe handler and parse the
>>> elf memory dump.
>>
>> Yeah - I may be doing something new here.  Android uses ptrace
>> in debuggerd, which is their crash reporting tool, but they wake
>> it up with signals before the dying program goes into coredump.
> 
> I think ptrace API does not provide guarantees that it is possible
> to attach to the process when it coredumps.
> 
> It might work in current kernels, but might break in new ones.

If it's not too much trouble, it would be nice to continue
this behaviour.   Admittedly, my style of crash handling appears
to be new, and I don't want to unnecessarily burden the code,
but so far it works really well, and currently requires very
minimal change to the existing ptrace code.  One thing that's
nice about what I'm doing, is that I don't rely on the
whole signal state machine of the process to interact with it
(since a dying process can't respond correctly).

So hopefully, continuing to support ptrace for a dying process
won't interfere or burden the existing (rather complex)
state processing in the current code.

Just for reference, below is the patch I settled on for my own kernel.
I'm planning taking a look at the PTRACE_SEIZE code to see if it
accomplishes what I need, but haven't done that yet.

I do have a question, though - how will a tracer know that it can
use PTRACE_SEIZE?  Is there some introspection API?  My code will
be running mainly against a patched 3.0 for some time (a few years),
but if I could make it interoperate with a kernel that supports
PTRACE_SEIZE (when it is mainlined), that would be great.

 -- Tim

commit dd54b901759428e60ed57b2d6cb77d25a8db767f
Author: tbird <tim.bird@am.sony.com>
Date:   Tue Feb 28 14:13:08 2012 -0800

    Support ptrace_attach with no signal side-effects.

    In the normal case, a ptrace_attach operation will convert
    a process to TASK_TRACED by sending it a SIGSTOP signal,
    after setting task->ptrace.  This won't work on a dying
    process because during do_coredump(), the dying process won't
    process the STOP signal and change state.

    Modify ptrace_attach() so that the tracee task state is modified
    directly.  This allows subsequent ptrace_check_attach() calls
    to work correctly, and avoids having a pending SIGSTOP signal
    on the tree process (which interferes with waiting for
    the core pipe handler).

    Note that a more full-featured implementation of this is in the
    works (as of March, 2012) by Tejun Heo, called PTRACE_SEIZE.
    Once that gets mainlined, this patch may not be needed, or might
    need to be reworked.

    Signed-off-by: Tim Bird <tim.bird@am.sony.com>

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 26147d1..9c7bf8e 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -223,7 +223,16 @@ static int ptrace_attach(struct task_struct *task)
 		task->ptrace |= PT_PTRACE_CAP;

 	__ptrace_link(task, current);
-	send_sig_info(SIGSTOP, SEND_SIG_FORCED, task);
+	
+	/*
+ 	 * If doing coredump, just convert directly to TASK_TRACED.
+ 	 * A dying process doesn't process signals normally.
+ 	 */
+	if (unlikely(task->mm->core_state)) {
+		set_task_state(task, TASK_TRACED);
+	} else {
+		send_sig_info(SIGSTOP, SEND_SIG_FORCED, task);
+	}

 	spin_lock(&task->sighand->siglock);


=============================
Tim Bird
Architecture Group Chair, CE Workgroup of the Linux Foundation
Senior Staff Engineer, Sony Network Entertainment
=============================