Just in case, please ignore

        [PATCH 54] PTRACE_SINGLESTEP shouldn't bypass SYSCALL_EXIT
        [PATCH 55] workaround to pass the assert() at step-from-clone.c:193

patches. You were right, there are ugly even as temporary changes.
And unneeded.

Currently the code wrongly assumes that PTRACE_SINGLESTEP should
trigger SIGTRAP only after return to user-mode. But if the tracee
was stopped somewhere inside syscall we should report SIGTRAP before
we return to the user-mode, otherwize we can't correctly step over
syscall instruction.

The vanilla kernel relies on syscall_trace_leave() which does
send_sigtrap(TRAP_BRKPT). But with utrace-ptrace the tracee sleeps
in do_notify_resume() path, syscall_trace_leave() won't be called.

Change ptrace_resume_ck_syscall() to do send_sigtrap(TRAP_BRKPT)
if context->resume != UTRACE_RESUME and we are going to return
to user-mode.

This looks obviously correct and simple, can't understand why I spent
sooooooooo much time doing this fix.

Do you see any problems?

IOW, every time we need to check whether we should generate/report
the artificial SYSCALL_EXIT stop event, we must also check whether
we need send_sigtrap().

With this patch we pass the new step-from-clone.c test.
        
Further changes:

        - I guess, we should also send SIGTRAP when SINGLESTEP resumes
          the tracee from SYSCALL_EXIT stop. This is trivial, but I need
          to write the test-case first.

        - perhaps it makes sense to change force_sig_info() to use
          lock_task_sighand(), then we can avoid taking taskist around
          send_sigtrap().

---

 kernel/ptrace.c |   15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

--- PU/kernel/ptrace.c~59_STEP_SEND_SIGTRAP     2009-09-24 22:08:22.000000000 
+0200
+++ PU/kernel/ptrace.c  2009-09-25 02:45:17.000000000 +0200
@@ -289,15 +289,27 @@ static u32 ptrace_report_syscall_exit(en
        return UTRACE_STOP;
 }
 
+static void ptrace_send_sigtrap(struct task_struct *tracee)
+{
+       read_lock(&tasklist_lock);
+       if (tracee->sighand)
+               send_sigtrap(tracee, task_pt_regs(tracee), 0, TRAP_BRKPT);
+       read_unlock(&tasklist_lock);
+}
+
 static void ptrace_resume_ck_syscall(struct utrace_engine *engine,
                                struct task_struct *tracee, long data)
 {
        struct ptrace_context *context = ptrace_context(engine);
 
-       if (context->options & PTRACE_O_TRACE_SYSCALL) {
-               if (ev_empty(context))
-                       push_syscall_event(context);
-       }
+       if (!ev_empty(context))
+               return;
+
+       if (context->options & PTRACE_O_TRACE_SYSCALL)
+               push_syscall_event(context);
+
+       if (context->resume != UTRACE_RESUME)
+               ptrace_send_sigtrap(tracee);
 }
 
 static u32 ptrace_report_exec(enum utrace_resume_action action,
@@ -969,6 +981,8 @@ static void do_ptrace_resume(struct utra
        else
                context->options &= ~PTRACE_O_TRACE_SYSCALL;
 
+       context->resume = action;
+
        if (!ev_empty(context)) {
                struct ptrace_event *ev = ev_pop(context);
 
@@ -986,7 +1000,6 @@ static void do_ptrace_resume(struct utra
                }
        }
 
-       context->resume = action;
        ptrace_wake_up(engine, tracee, action);
 }
 

Reply via email to