[PATCH 34] cleanup/simplify stop/resume mess
Oleg Nesterov
oleg at redhat.com
Tue Sep 15 18:48:51 UTC 2009
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;
}
More information about the utrace-devel
mailing list