Re: ptrace single-stepping change breaks Wine

[Linus Torvalds <torvalds@osdl.org>]

On Wed, 29 Dec 2004, Davide Libenzi wrote:
> 
> I think same. My test simply let the function processing to let thru and 
> reach the fake signal sending point.

No, your test-case doesn't even send a signal at all, because your
test-program just uses a PTRACE_SINGLESTEP without any "send signal" - so
"exit_code" will be zero after the debuggee gets released from the
"ptrace_notify()", and the suspect code will never be executed..

The problem I think I see (and which the comment alludes to) is that if
the controlling process continues the debuggee with a signal, we'll be
doing the wrong thing: the code in do_syscall_trace() will take that
signal (in "current->exit_code") and send it as a real signal to the
debuggee, but since it is debugged, it will be caught (again) by the
controlling process, which apparently in the case of Wine gets very
confused.

So I _think_ the problem happens for the following schenario:
 - wine for some reason does a PTRACE_SINGLESTEP over a system call
 - after the single-step, wine ends up trying to get the sub-process to 
   enter a signal handler with ptrace( PTRACE_CONT, ... sig)
 - the normal ptrace path (where we trap a signal - see kernel/signal.c 
   and the "ptrace_stop()" logic) handles this correctly, but 
   do_syscall_trace() will do a "send_sig()"
 - we'll try to handle the signal when returning to the traced process
 - now "get_signal_to_deliver()" will invoce the ptrace logic AGAIN, and 
   call ptrace_stop() for this fake signal, and we'll report a new event 
   to the controlling process.

Does this make sense?

If so, we have a few options:

 - very hacky one: teach all ptrace users that sometimes the signal you
   continue with can be reflected back at you, and you should just "cont
   signr"  _again_.

   This is a really bad option, partly because it's hard to tell when it's 
   just a spurious reflected signal, partly because there are many ptrace 
   users, and to a large part just because it's clearly too hacky. But it
   might be a valid approach for a Wine person who is used to Wine, and
   wants to verify whether this is indeed the schenario that triggers the
   problem..

 - Stupid approach: mark the siginfo that we send as the fake one as being 
   reflected, and have get_signal_to_deliver() not apply the ptrace 
   stopping to that.

 - Possibly cleaner approach: make system call tracing just use a proper
   SIGTRAP in the first place, and always handle all the ptrace_stop() etc
   cruft in kernel/signal.c like it handles all other calls.

I dunno. The final one looks fairly simple and clean, something like the
following, but I'm most likely overlooking some reason why this won't
work..

(And as usual, this patch has not been tested in any shape, way or form. 
In fact, it hasn't even seen an x86 machine, since I edited it out as a 
fake on my ppc64.. Somebody with more brains than me should actually try 
to get it to work)

		Linus

----
===== arch/i386/kernel/ptrace.c 1.28 vs edited =====
--- 1.28/arch/i386/kernel/ptrace.c	2004-11-22 09:44:52 -08:00
+++ edited/arch/i386/kernel/ptrace.c	2004-12-29 23:21:58 -08:00
@@ -559,6 +559,8 @@
 __attribute__((regparm(3)))
 void do_syscall_trace(struct pt_regs *regs, int entryexit)
 {
+	struct siginfo info;
+
 	if (unlikely(current->audit_context)) {
 		if (!entryexit)
 			audit_syscall_entry(current, regs->orig_eax,
@@ -573,18 +575,18 @@
 		return;
 	if (!(current->ptrace & PT_PTRACED))
 		return;
+
+	memset(&info, 0, sizeof(info));
+	info.si_signo = SIGTRAP;
+
 	/* the 0x80 provides a way for the tracing parent to distinguish
 	   between a syscall stop and SIGTRAP delivery */
-	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD) &&
-				 !test_thread_flag(TIF_SINGLESTEP) ? 0x80 : 0));
+	info.si_code = SIGTRAP;
+	if ((current->ptrace & PT_TRACESYSGOOD) && !test_thread_flag(TIF_SINGLESTEP))
+		info.si_code = SIGTRAP | 0x80;
+	info.si_pid = current->tgid;
+	info.si_uid = current->uid;
 
-	/*
-	 * this isn't the same as continuing with a signal, but it will do
-	 * for normal use.  strace only continues with a signal if the
-	 * stopping signal is not SIGTRAP.  -brl
-	 */
-	if (current->exit_code) {
-		send_sig(current->exit_code, current, 1);
-		current->exit_code = 0;
-	}
+	/* Send us the fakey SIGTRAP */
+	send_sig_info(SIGTRAP, &info, current);
 }