copy_process && utrace_init_task (Was: [PATCH 133] stepping, accommodate to utrace-cleanup changes)

[Oleg Nesterov]

On 11/16, Oleg Nesterov wrote:
>
> On 11/16, Oleg Nesterov wrote:
> >
> > And I didn't check "make xcheck", I guess it still crashes the kernel.
>
> Yes it does. I am almost sure the bug should be trivial, but
> somehow can't find find it.

Found the trivial but nasty problem.

tracehook_finish_clone()->utrace_init_task() sets ->utrace = NULL, but
this is too late. If copy_process() fails before, tracehook_free_task()
will free parent's ->utrace copied by dup_task_struct().

This is nasty because we need some changes outside of utrace/tracehook
files. Perhaps we should send something like the patch below to Andrew
right now?


And! While this bug could perfectly explain the crash, it doesn't.
I appiled this patch

	--- UTRACE-PTRACE/kernel/fork.c~XXX_CRASH	2009-11-16 20:26:23.000000000 +0100
	+++ UTRACE-PTRACE/kernel/fork.c	2009-11-17 20:33:50.000000000 +0100
	@@ -1019,6 +1019,7 @@ static struct task_struct *copy_process(
		if (!p)
			goto fork_out;
	 
	+p->utrace = NULL;
		ftrace_graph_init_task(p);
	 
		rt_mutex_init_task(p);

but "make xcheck" still crashes. Still investigating.

Oleg.

--- TH/include/linux/ptrace.h~TH_INIT_TASK	2009-11-10 21:31:25.000000000 +0100
+++ TH/include/linux/ptrace.h	2009-11-17 20:43:42.000000000 +0100
@@ -148,6 +148,14 @@ static inline int ptrace_event(int mask,
 	return 1;
 }
 
+static inline void ptrace_init_task(struct task_struct *child)
+{
+	INIT_LIST_HEAD(&child->ptrace_entry);
+	INIT_LIST_HEAD(&child->ptraced);
+	child->parent = child->real_parent;
+	child->ptrace = 0;
+}
+
 /**
  * ptrace_init_task - initialize ptrace state for a new child
  * @child:		new child task
@@ -158,12 +166,8 @@ static inline int ptrace_event(int mask,
  *
  * Called with current's siglock and write_lock_irq(&tasklist_lock) held.
  */
-static inline void ptrace_init_task(struct task_struct *child, bool ptrace)
+static inline void ptrace_finish_clone(struct task_struct *child, bool ptrace)
 {
-	INIT_LIST_HEAD(&child->ptrace_entry);
-	INIT_LIST_HEAD(&child->ptraced);
-	child->parent = child->real_parent;
-	child->ptrace = 0;
 	if (unlikely(ptrace) && (current->ptrace & PT_PTRACED)) {
 		child->ptrace = current->ptrace;
 		__ptrace_link(child, current->parent);
--- TH/include/linux/tracehook.h~TH_INIT_TASK	2009-11-11 21:49:42.000000000 +0100
+++ TH/include/linux/tracehook.h	2009-11-17 20:42:43.000000000 +0100
@@ -247,6 +247,11 @@ static inline int tracehook_prepare_clon
 	return 0;
 }
 
+static inline void tracehook_init_task(struct task_struct *child)
+{
+	ptrace_init_task(child);
+}
+
 /**
  * tracehook_finish_clone - new child created and being attached
  * @child:		new child task
@@ -261,7 +266,7 @@ static inline int tracehook_prepare_clon
 static inline void tracehook_finish_clone(struct task_struct *child,
 					  unsigned long clone_flags, int trace)
 {
-	ptrace_init_task(child, (clone_flags & CLONE_PTRACE) || trace);
+	ptrace_finish_clone(child, (clone_flags & CLONE_PTRACE) || trace);
 }
 
 /**
--- TH/kernel/fork.c~TH_INIT_TASK	2009-11-07 22:15:15.000000000 +0100
+++ TH/kernel/fork.c	2009-11-17 20:40:52.000000000 +0100
@@ -1018,8 +1018,8 @@ static struct task_struct *copy_process(
 	if (!p)
 		goto fork_out;
 
+	tracehook_init_task(p);
 	ftrace_graph_init_task(p);
-
 	rt_mutex_init_task(p);
 
 #ifdef CONFIG_PROVE_LOCKING