The code still needs comments. I'll try to explain how the code should
work at least here.

        struct ptrace_event {
                int             ev_code;
                unsigned long   ev_message;
                resume_func_t   ev_resume;
        };

This represents the state of the tracee when it stops,

        ->ev_code       == tracee->exit_code
        ->ev_message    == tracee->ptrace_message
        ->ev_resume     == non-NULL if we need some special action on resume,
                           for example syscall entry/exit do send_sig()


struct ptrace_context has a simple FIFO

        struct ptrace_event ev_array[2];

and two simple helpers:

        ev_push()       - for tracee, add the new event
        ev_pop()        - for tracer, dequeue the first event


For example, lets see what happens when the tracee reports syscall_exit.
ptrace_report_syscall_exit() simply does ev_push() and that is all.
No need to worry in case this event is stacked. Say, we may have
PTRACE_EVENT_EXEC which should be reported first.

When the tracee actually stops, ptrace_notify_stop() looks at the
first ptrace_event, setups ->exit_code and notifies the tracer.

ptrace_resume() does ev_push(), calls ->ev_resume() if it is !NULL.
If we have more events, it calls ptrace_notify_stop() to trick ptrace
into thinking the tracee stops again, otherwise it wakes up the tracee.

I think this is more readable and clean. Further changes:

        - just noticed, do_notify_parent_cldstop() is not safe
          when called by tracer. The tracee can be killed, released
          by the tracer's subthread, tracee->parent can exit and
          its memory can be freed.

        - when we report the stacked ptrace_event, we must validate
          it is still valid. The tracer can change options in between,
          or it can use PTRACE_CONT which (if I understand correctly)
          should "disable" the report from syscall_exit.

        - when the tracee reports, say, PTRACE_EVENT_EXEC and the
          tracer does PTRACE_SYSCALL we should report syscall-exit,
          even in case the tracee has stopped in utrace_resume()
          before return to user-mode.

---

 kernel/ptrace.c |   94 +++++++++++++++++++++++++-------------------------------
 1 file changed, 43 insertions(+), 51 deletions(-)

--- PU/kernel/ptrace.c~34_CLEANUP_RESUME        2009-09-15 10:34:59.000000000 
+0200
+++ PU/kernel/ptrace.c  2009-09-15 14:49:47.000000000 +0200
@@ -93,10 +93,6 @@ void __ptrace_link(struct task_struct *c
 static const struct utrace_engine_ops ptrace_utrace_ops; /* forward decl */
 static int ptrace_attach_task(struct task_struct *tracee, int options);
 static void ptrace_abort_attach(struct task_struct *tracee);
-static void ptrace_wake_up(struct utrace_engine *engine,
-               struct task_struct *tracee, enum utrace_resume_action action);
-static void do_ptrace_notify_stop(struct ptrace_context *context,
-                                       struct task_struct *tracee);
 
 static void ptrace_detach_task(struct task_struct *child, int sig)
 {
@@ -262,18 +258,6 @@ static void ptrace_clone_attach(struct t
        set_tsk_thread_flag(child, TIF_SIGPENDING);
 }
 
-static void ptrace_resume_vfork_done(struct utrace_engine *engine,
-                               struct task_struct *tracee, long data)
-{
-        ptrace_wake_up(engine, tracee, UTRACE_RESUME);
-}
-
-static void ptrace_resume_clone(struct utrace_engine *engine,
-                               struct task_struct *tracee, long data)
-{
-       ptrace_wake_up(engine, tracee, UTRACE_RESUME);
-}
-
 static u32 ptrace_report_clone(enum utrace_resume_action action,
                               struct utrace_engine *engine,
                               struct task_struct *parent,
@@ -312,7 +296,6 @@ static u32 ptrace_report_clone(enum utra
                struct ptrace_event *ev = ev_push(context);
 
                ev->ev_message = child->pid;
-               ev->ev_resume = ptrace_resume_clone;
                ev->ev_code = (event << 8) | SIGTRAP;
 
                ret = UTRACE_STOP;
@@ -323,7 +306,6 @@ static u32 ptrace_report_clone(enum utra
                struct ptrace_event *ev = ev_push(context);
 
                ev->ev_message = child->pid;
-               ev->ev_resume = ptrace_resume_vfork_done;
                ev->ev_code = (PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP;
 
                ret = UTRACE_STOP;
@@ -357,31 +339,6 @@ static u32 ptrace_report_syscall_entry(u
        return ptrace_report_syscall(UTRACE_SYSCALL_RUN, engine, task);
 }
 
-// XXX: move this all into ptrace_resume()
-static void ptrace_wake_up(struct utrace_engine *engine,
-                               struct task_struct *tracee,
-                               enum utrace_resume_action action)
-{
-       struct ptrace_context *context = ptrace_context(engine);
-       unsigned long flags;
-
-       if (!ev_empty(context)) {
-               do_ptrace_notify_stop(context, tracee);
-               return;
-       }
-
-       /* preserve the compatibility bug */
-       if (!lock_task_sighand(tracee, &flags))
-               return;
-       tracee->signal->flags &= ~SIGNAL_STOP_STOPPED;
-       unlock_task_sighand(tracee, &flags);
-
-       // XXX: FIXME!!! racy.
-       tracee->exit_code = 0;
-
-       utrace_control(tracee, engine, action);
-}
-
 static void ptrace_resume_syscall(struct utrace_engine *engine,
                                struct task_struct *tracee, long data)
 {
@@ -392,8 +349,6 @@ static void ptrace_resume_syscall(struct
                        send_sig(data, tracee, 1);
                read_unlock(&tasklist_lock);
        }
-
-       ptrace_wake_up(engine, tracee, UTRACE_RESUME);
 }
 
 static u32 ptrace_report_syscall_exit(enum utrace_resume_action action,
@@ -1002,6 +957,7 @@ static void do_ptrace_notify_stop(struct
        WARN_ON(ev->ev_code == 0);
        ev->ev_code = 0;
 
+       // XXX: !!!!!!!! UNSAFE when called by tracer !!!!!!!!!!!!!
        read_lock(&tasklist_lock);
        do_notify_parent_cldstop(tracee, CLD_TRAPPED);
        read_unlock(&tasklist_lock);
@@ -1033,6 +989,46 @@ void ptrace_notify_stop(struct task_stru
        }
 }
 
+static void ptrace_wake_up(struct utrace_engine *engine,
+                               struct task_struct *tracee,
+                               enum utrace_resume_action action)
+{
+       unsigned long flags;
+
+       /* preserve the compatibility bug */
+       if (!lock_task_sighand(tracee, &flags))
+               return;
+       tracee->signal->flags &= ~SIGNAL_STOP_STOPPED;
+       unlock_task_sighand(tracee, &flags);
+
+       // XXX: FIXME!!! racy.
+       tracee->exit_code = 0;
+
+       utrace_control(tracee, engine, action);
+}
+
+static void do_ptrace_resume(struct utrace_engine *engine,
+                               struct task_struct *tracee,
+                               long data)
+{
+       struct ptrace_context *context = ptrace_context(engine);
+
+       if (!ev_empty(context)) {
+               struct ptrace_event *ev = ev_pop(context);
+
+               WARN_ON(ev->ev_code); // XXX: debug
+               if (ev->ev_resume)
+                       ev->ev_resume(engine, tracee, data);
+
+               if (!ev_empty(context)) {
+                       do_ptrace_notify_stop(context, tracee);
+                       return;
+               }
+       }
+
+       ptrace_wake_up(engine, tracee, UTRACE_RESUME);
+}
+
 static int ptrace_resume(struct task_struct *child, long request, long data)
 {
        struct utrace_engine *engine;
@@ -1083,12 +1079,7 @@ static int ptrace_resume(struct task_str
                int event;
 
                if (!ev_empty(context)) {
-                       struct ptrace_event *ev = ev_pop(context);
-
-                       WARN_ON(ev->ev_code); // XXX: debug
-                       ev->ev_resume(engine, child, data);
-                       ev->ev_resume = NULL; // XXX: debug
-
+                       do_ptrace_resume(engine, child, data);
                        utrace_engine_put(engine);
                        return 0;
                }

Reply via email to