Issue with change to CLONE_THREAD and CLONE_PARENT

As you may or may not be aware, there is a project here at IBM to do a more POSIX-compliant pthread library for Linux, called NGPT. While I am not part of the core project, I have been asked to help them with kernel issues.

This library uses thread groups to manage the kernel tasks it creates. It was working all right until you introduced a change to how CLONE_THREAD works in 2.4.7 by making CLONE_THREAD imply CLONE_PARENT. This change has some undesirable side effects.

I understand the model you're aiming for, where the task created by the original fork() is a monitor task and isn't part of the thread group. However this also means that the pid returned to the parent from fork() does not match what getpid() returns to the tasks inside the thread group. It also means it's no longer possible to use any model other than the one you envision.

The original bug reported that this change was intended to fix, where tasks can be parented to themselves, is not in fact fixed by this change. It merely masks the problem by making it less likely to happen. The real bug is in forget_original_parent in kernel/exit.c, where it's attempting to reparent children of the dying task to another task in the thread group. The bug is that it never checks to make sure the task it's reparenting is not the new parent.

Given that it's simple and straightforward to specify CLONE_THREAD and CLONE_PARENT together to produce the behavior you're looking for, I'm requesting that they be decoupled so those of us who don't want that behavior can still get it. I've included a patch below that backs that change out and also fixes the problem in forget_original_parent.

Dave McCracken

======================================================================
Dave McCracken IBM Linux Base Kernel Team 1-512-838-3059
[email protected] T/L 678-3059

------------

@@ -170,7 +170,13 @@

			/* We dont want people slaying init */
			p->exit_signal = SIGCHLD;
			p->self_exec_id++;

- p->p_opptr = reaper;
+
+ /* Make sure we're not reparenting to ourselves */
+ if (p == reaper)
+ p->p_opptr = child_reaper;
+ else
+ p->p_opptr = reaper;
+

			if (p->pdeath_signal) send_sig(p->pdeath_signal, p, 0);
		}
	}

@@ -703,7 +703,7 @@

	/* CLONE_PARENT and CLONE_THREAD re-use the old parent */
	p->p_opptr = current->p_opptr;
	p->p_pptr = current->p_pptr;
-	if (!(clone_flags & (CLONE_PARENT | CLONE_THREAD))) {
+	if (!(clone_flags & CLONE_PARENT)) {
		p->p_opptr = current;
		if (!(p->ptrace & PT_PTRACED))
			p->p_pptr = current;