Re: [RFC PATCH] set TASK_TRACED before arch_ptrace code to fix a race

["Luming Yu" <luming.yu@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5131 bytes --]

Just have chance to re-check the problem. I have spotted the place that strace
misbehaves when SIGTRAP is blocked by task being straced with -f flag
(follow trace)
The problem is that after debugee blocking SIGTRAP the task's exec
path would not wake up debugger (strace) with this signal. But strace
don't know about it, and expect such a wake up. Upon receiving a
subsequent wake up,strace still thought it was a wake up from "SIGTRAP
in previous exec path". From now on, Debuger and Debugee misunderstand
each other..

With the test case posted before in this thread and a customized
kernel, I got the following debug info from kernel:

syscall_trace_enter exiting (4717)
report_exec:pid= 4717 parent=4715, real_parent=4716,current->ptrace =00000001,
report_exec:pid= 4717 parent=4715, real_parent=4716,current->ptrace =00000001,
syscall_trace_leave entering (4717, syscall:1033)
 ..sig_ignored=1..
4715, PTRACE_PEEKUSR 4717 @addr 00000830 return 00000000
4715, PTRACE_PEEKUSR 4717 @addr 000008c0 return 00000000
4715, PTRACE_PEEKUSR 4717 @addr 000008d0 return 00000000
4715, PTRACE_PEEKUSR 4717 @addr 000008d0 return 00000000
4715, PTRACE_PEEKUSR 4717 @addr 000008c0 return 00000000
syscall_trace current(4717)->exit_code(0)
syscall_trace_leave exiting (4717)
syscall_trace_enter beginning (4717, syscall: 1060)
 ..sig_ignored=1..
4715, PTRACE_PEEKUSR 4717 @addr 00000830 return 00000010
4715, PTRACE_PEEKUSR 4717 @addr 000008b8 return 00000424
syscall_trace current(4717)->exit_code(0)
syscall_trace_enter exiting (4717)

Delete the line that blocks SIGTRAP in the test case, rerun it, I got:

syscall_trace current(4724)->exit_code(0)
syscall_trace_enter exiting (4724)
report_exec:pid= 4724 parent=4722, real_parent=4723,current->ptrace =00000001,
report_exec:pid= 4724 parent=4722, real_parent=4723,current->ptrace =00000001,
syscall_trace_leave entering (4724, syscall:1033)
 ..sig_ignored=1..
4722, PTRACE_PEEKUSR 4724 @addr 00000830 return 00000000
4722, PTRACE_PEEKUSR 4724 @addr 000008c0 return 00000000
4722, PTRACE_PEEKUSR 4724 @addr 000008d0 return 00000000
4722, PTRACE_PEEKUSR 4724 @addr 000008d0 return 00000000
4722, PTRACE_PEEKUSR 4724 @addr 000008c0 return 00000000
syscall_trace current(4724)->exit_code(0)
syscall_trace_leave exiting (4724)
 ..sig_ignored=1..
4722, PTRACE_PEEKUSR 4724 @addr 00000830 return 00000000
4722, PTRACE_PEEKUSR 4724 @addr 000008b8 return 00000409
syscall_trace_enter beginning (4724, syscall: 1060)
 ..sig_ignored=1..


The attahced patch is a proposal for this problem. But I still don't know
why it is just specific to IA64.

***Note: The following patch is copy&paste, If you want apply, please
check out the attached version


Signed-off-by: Yu Luming <luming.yu@intel.com>
--------------------------------------
 include/linux/tracehook.h |    4 +++-
 kernel/signal.c           |   11 ++++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff -Bru 0/include/linux/tracehook.h 1/include/linux/tracehook.h
--- 0/include/linux/tracehook.h 2008-08-18 20:43:21.000000000 -0400
+++ 1/include/linux/tracehook.h 2008-09-09 09:51:14.000000000 -0400
@@ -195,8 +195,10 @@
                                         struct pt_regs *regs)
 {
        if (!ptrace_event(PT_TRACE_EXEC, PTRACE_EVENT_EXEC, 0) &&
-           unlikely(task_ptrace(current) & PT_PTRACED))
+           unlikely(task_ptrace(current) & PT_PTRACED)){
                send_sig(SIGTRAP, current, 0);
+               set_tsk_thread_flag(current,TIF_SIGPENDING);
+       }
 }

 /**
diff -Bru 0/kernel/signal.c 1/kernel/signal.c
--- 0/kernel/signal.c   2008-08-18 20:43:21.000000000 -0400
+++ 1/kernel/signal.c   2008-09-09 09:50:38.000000000 -0400
@@ -384,8 +384,14 @@
 static int __dequeue_signal(struct sigpending *pending, sigset_t *mask,
                        siginfo_t *info)
 {
-       int sig = next_signal(pending, mask);
+       int sig = 0;
+       int allow_trap = 0;

+       if((current->ptrace & PT_PTRACED) && sigismember(mask,SIGTRAP)){
+               sigdelset(mask,SIGTRAP);
+               allow_trap = 1;
+       }
+       sig = next_signal(pending, mask);
        if (sig) {
                if (current->notifier) {
                        if (sigismember(current->notifier_mask, sig)) {
@@ -399,6 +405,9 @@
                collect_signal(sig, pending, info);
        }

+       if(allow_trap)
+               sigaddset(mask,SIGTRAP);
+
        return sig;
}

On Fri, Jun 6, 2008 at 8:07 AM, Roland McGrath <roland@redhat.com> wrote:
>> I _guess_ this is caused by the fact that test2.sh is a shell script, so
>> the kernel executes the shell, and maybe utrace produces a second execve
>> notifications in this case? Roland, can you shed some light?
>
> Not really.  The utrace kernels Luming is trying are intended to match the
> vanilla ptrace behavior.  I don't think it's very useful to worry about the
> difference between some utrace kernel and the current vanilla kernel.
> Let's just look at what the current vanilla kernel is doing and compare
> that to what an older vanilla kernel did if older versions produced
> different results for the test case.
>
>
> Thanks,
> Roland
>

[-- Attachment #2: 44620-upstream.patch --]
[-- Type: application/octet-stream, Size: 1251 bytes --]

diff -Bru 0/include/linux/tracehook.h 1/include/linux/tracehook.h
--- 0/include/linux/tracehook.h	2008-08-18 20:43:21.000000000 -0400
+++ 1/include/linux/tracehook.h	2008-09-09 09:51:14.000000000 -0400
@@ -195,8 +195,10 @@
 					 struct pt_regs *regs)
 {
 	if (!ptrace_event(PT_TRACE_EXEC, PTRACE_EVENT_EXEC, 0) &&
-	    unlikely(task_ptrace(current) & PT_PTRACED))
+	    unlikely(task_ptrace(current) & PT_PTRACED)){
 		send_sig(SIGTRAP, current, 0);
+		set_tsk_thread_flag(current,TIF_SIGPENDING);
+	}
 }
 
 /**
diff -Bru 0/kernel/signal.c 1/kernel/signal.c
--- 0/kernel/signal.c	2008-08-18 20:43:21.000000000 -0400
+++ 1/kernel/signal.c	2008-09-09 09:50:38.000000000 -0400
@@ -384,8 +384,14 @@
 static int __dequeue_signal(struct sigpending *pending, sigset_t *mask,
 			siginfo_t *info)
 {
-	int sig = next_signal(pending, mask);
+	int sig = 0;
+	int allow_trap = 0;
 
+	if((current->ptrace & PT_PTRACED) && sigismember(mask,SIGTRAP)){
+		sigdelset(mask,SIGTRAP);
+		allow_trap = 1;
+	}
+	sig = next_signal(pending, mask);
 	if (sig) {
 		if (current->notifier) {
 			if (sigismember(current->notifier_mask, sig)) {
@@ -399,6 +405,9 @@
 		collect_signal(sig, pending, info);
 	}
 
+	if(allow_trap)
+		sigaddset(mask,SIGTRAP);
+
 	return sig;
 }