[PATCH 08] change ptrace_traceme() to use the new helpers, kill prepare/finish attach

[Oleg Nesterov]

Change ptrace_traceme() to use the new helpers. Kill the now unused
prepare_ptrace_attach/finish_ptrace_attach.

finish_ptrace_attach(ret) logic was wrong. If ->parent has PF_EXITING
we must UTRACE_DETACH but ret == 0. Fix this.

Note: we attach the new engine unconditionally, before security check.
Not good, another task can do ptrace_attach() and get EPERM. Hopefully
we can live with this race, or somehow fix it later.

But security_ptrace_traceme() is strange anyway, I never understood it.
It is called under tasklist_lock, but I can't see how this lock can
help. ->parent can change its creds right after the check.


Somehow with the last 2 patches the kernel survives after make xcheck.
A lot of warning from ptrace_resumed()->WARN_ON(task->last_siginfo != info)
but forget_original_parent()->BUG_ON(task_ptrace(p)) has gone away.
I don't understand how this was "fixed", this means I missed something :/


I am ignoring ptrace_report_clone() for now. I think it should not
rely on tracehook_finish_clone/tracehook_report_clone(). Instead it
should do all work itself, we have PF_STARTING. In that case it can
share the code with ptrace_traceme().

---

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

--- PU/kernel/ptrace.c~08_TRACEME	2009-08-17 15:31:07.000000000 +0200
+++ PU/kernel/ptrace.c	2009-08-17 16:05:21.000000000 +0200
@@ -464,52 +464,6 @@ static const struct utrace_engine_ops pt
 };
 
 /*
- * Detach the utrace engine on error.  On success, set @engine->data to
- * the ptracer's task_struct pointer.  This distinguishes an engine for
- * attached ptrace from an engine left behind after a PTRACE_DETACH call
- * that left a parting signal to deliver.
- */
-static int finish_ptrace_attach(struct task_struct *task,
-				struct utrace_engine *engine,
-				int retval)
-{
-	if (retval) {
-		int error = utrace_control(task, engine, UTRACE_DETACH);
-		WARN_ON(error && error != -ESRCH && error != -EALREADY);
-	}
-
-	utrace_engine_put(engine);
-	return retval;
-}
-
-/*
- * Attach a utrace engine for ptrace and set up its event mask.
- * Returns the engine pointer or an IS_ERR() pointer.
- */
-static struct utrace_engine *prepare_ptrace_attach(
-	struct task_struct *child, struct task_struct *parent)
-{
-	struct utrace_engine *engine;
-
-	engine = utrace_attach_task(child, UTRACE_ATTACH_CREATE |
-				    UTRACE_ATTACH_EXCLUSIVE |
-				    UTRACE_ATTACH_MATCH_OPS,
-				    &ptrace_utrace_ops, NULL);
-
-	if (IS_ERR(engine)) {
-		if (engine != ERR_PTR(-ESRCH) &&
-		    engine != ERR_PTR(-ERESTARTNOINTR))
-			engine = ERR_PTR(-EPERM);
-	} else {
-		int ret = ptrace_update_utrace(child, engine);
-		if (ret)
-			engine = ERR_PTR(finish_ptrace_attach(child, engine,
-							      -ESRCH));
-	}
-	return engine;
-}
-
-/*
  * Attach a utrace engine for ptrace and set up its event mask.
  * Returns error code or 0 on success.
  */
@@ -688,13 +642,13 @@ out:
  */
 int ptrace_traceme(void)
 {
-	int ret = -EPERM;
-	struct utrace_engine *engine;
+	bool detach = true;
+	int ret = ptrace_attach_task(current);
 
-	engine = prepare_ptrace_attach(current, current->parent);
-	if (unlikely(IS_ERR(engine)))
-		return PTR_ERR(engine);
+	if (unlikely(ret))
+		return ret;
 
+	ret = -EPERM;
 	write_lock_irq(&tasklist_lock);
 	/* Are we already being traced? */
 	if (!current->ptrace) {
@@ -707,11 +661,14 @@ int ptrace_traceme(void)
 		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);
 
-	return finish_ptrace_attach(current, engine, ret);
+	if (detach)
+		ptrace_abort_attach(current);
+	return ret;
 }
 
 /*