Until we kill ->ptrace, attach must ensure ->ptrace == 0 to avoid the
races with detach. We can move these checks to ptrace_attach_task()
from the callers, and we can do this check lockless.
>From now ptrace_abort_attach() is only called when the tracee had no
chance to execute any callback, a simple UTRACE_DETACH is enough.
ptrace_attach() doesn't need ptrace_abort_attach() at all, if the
tracee is killed we don't care about attached engine.
---
kernel/ptrace.c | 67 ++++++++++++++++++++++++++++++--------------------------
1 file changed, 36 insertions(+), 31 deletions(-)
--- PU/kernel/ptrace.c~09_MV_PTRACE_CK 2009-08-19 16:49:25.000000000 +0200
+++ PU/kernel/ptrace.c 2009-08-19 17:41:18.000000000 +0200
@@ -471,35 +471,45 @@ static int ptrace_attach_task(struct tas
{
struct utrace_engine *engine;
unsigned long events;
+ int err = -EPERM;
engine = utrace_attach_task(tracee, UTRACE_ATTACH_CREATE |
UTRACE_ATTACH_EXCLUSIVE |
UTRACE_ATTACH_MATCH_OPS,
&ptrace_utrace_ops, NULL);
if (unlikely(IS_ERR(engine))) {
- int err = PTR_ERR(engine);
- if (err != -ESRCH && err != -ERESTARTNOINTR)
- err = -EPERM ;
+ if (PTR_ERR(engine) == -ESRCH ||
+ PTR_ERR(engine) == -ERESTARTNOINTR)
+ err = PTR_ERR(engine);
return err;
}
+
/*
- * We need QUIESCE for resume handling, CLONE to check
- * for CLONE_PTRACE, other events are always reported.
- */
- events = UTRACE_EVENT(QUIESCE) | UTRACE_EVENT(CLONE) |
- UTRACE_EVENT(EXEC) | UTRACE_EVENT_SIGNAL_ALL;
- /*
- * It can fail only if the tracee is dead, the caller
- * must notice this before setting PT_PTRACED.
+ * Check ->ptrace to make sure we don't race with the previous
+ * tracer which didn't finish detach. If it is clear, nobody
+ * can change it except us due to UTRACE_ATTACH_EXCLUSIVE.
*/
- utrace_set_events(tracee, engine, events);
+ if (likely(!tracee->ptrace)) {
+ /*
+ * We need QUIESCE for resume handling, CLONE to check
+ * for CLONE_PTRACE, other events are always reported.
+ */
+ events = UTRACE_EVENT(QUIESCE) | UTRACE_EVENT(CLONE) |
+ UTRACE_EVENT(EXEC) | UTRACE_EVENT_SIGNAL_ALL;
+ /*
+ * It can fail only if the tracee is dead, the caller
+ * must notice this before setting PT_PTRACED.
+ */
+ utrace_set_events(tracee, engine, events);
+ err = 0;
+ }
+
utrace_engine_put(engine);
- return 0;
+ return err;
}
static void ptrace_abort_attach(struct task_struct *tracee)
{
- /* XXX we raced with detach. Double check UTRACE_DETACH is enough */
ptrace_detach_task(tracee, 0);
}
@@ -613,9 +623,8 @@ int ptrace_attach(struct task_struct *ta
retval = -EPERM;
if (unlikely(task->exit_state))
goto unlock_tasklist;
- if (task->ptrace)
- goto unlock_tasklist;
+ BUG_ON(task->ptrace);
task->ptrace = PT_PTRACED;
if (capable(CAP_SYS_PTRACE))
task->ptrace |= PT_PTRACE_CAP;
@@ -626,8 +635,6 @@ int ptrace_attach(struct task_struct *ta
retval = 0;
unlock_tasklist:
write_unlock_irq(&tasklist_lock);
- if (retval)
- ptrace_abort_attach(task);
unlock_creds:
mutex_unlock(&task->cred_guard_mutex);
out:
@@ -650,19 +657,17 @@ int ptrace_traceme(void)
ret = -EPERM;
write_lock_irq(&tasklist_lock);
- /* Are we already being traced? */
- if (!current->ptrace) {
- ret = security_ptrace_traceme(current->parent);
- /*
- * Check PF_EXITING to ensure ->real_parent has not passed
- * exit_ptrace(). Otherwise we don't report the error but
- * pretend ->real_parent untraces us right after return.
- */
- if (!ret && !(current->real_parent->flags & PF_EXITING)) {
- current->ptrace = PT_PTRACED;
- __ptrace_link(current, current->real_parent);
- detach = false;
- }
+ BUG_ON(current->ptrace);
+ ret = security_ptrace_traceme(current->parent);
+ /*
+ * Check PF_EXITING to ensure ->real_parent has not passed
+ * exit_ptrace(). Otherwise we don't report the error but
+ * pretend ->real_parent untraces us right after return.
+ */
+ if (!ret && !(current->real_parent->flags & PF_EXITING)) {
+ current->ptrace = PT_PTRACED;
+ __ptrace_link(current, current->real_parent);
+ detach = false;
}
write_unlock_irq(&tasklist_lock);
