: mktree.c: cr_set_creator() question

Oren:

I was trying to run the ptree test I had posted earlier with v14-rc3 and get a SIGSEGV in mktree (when restarting a simple process tree).

The SIGSEGV is due to 'creator' being NULL @ mktree.c:585.

I debugged it a bit and was not clear on the following 'task-ppid == 1' check. It seems to check for orphaned processes using 'ppid == 1', but 'ppid == 1' could be 1 if the 'task' was an actual child of the container-init right ?

Also, 'phantom' does not seem to be set anywhere and so 'creator' ends up being NULL.

Since in my case, 'task' refers to an actual child of container-init, I reversed the order of the following two checks as a quick hack, and it seems to fix the SIGSEGV.

[v14-rc3] mktree.c: cr_set_creator() (lines 543-553)

        } else if (task->ppid == 1) {
                /* (non-session-leader) orphan: creator is dummy */
                cr_dbg("pid %d: orphan session %d\n", task->pid, task->sid);
                if (!session->phantom)
                        if (cr_placeholder_task(ctx, task) < 0)
                                return -1;
                creator = session->phantom;
        } else if (task->sid == parent->sid) {
                /* (non-session-leader) inherit: creator is parent */
                cr_dbg("pid %d: inherit sid %d\n", task->pid, task->sid);
                creator = parent;

Sukadev

On Wed, 8 Apr 2009, Sukadev Bhattiprolu wrote:

Indeed the bug was that cr_placeholder_task() forgot to also set 'session->phantom = holder'. This fixes the crash (added to next version).

index 08cd56a..2e0705d 100644
@@ -540,6 +540,10 @@ static int cr_set_creator(struct cr_ctx *ctx, struct task *task)
 		/* dead: creator is session leader */
 		cr_dbg("pid %d: task is dead\n", task->pid);
 		creator = session;
+	} else if (task->sid == parent->sid) {
+		/* (non-session-leader) inherit: creator is parent */
+		cr_dbg("pid %d: inherit sid %d\n", task->pid, task->sid);
+		creator = parent;
 	} else if (task->ppid == 1) {
 		/* (non-session-leader) orphan: creator is dummy */
 		cr_dbg("pid %d: orphan session %d\n", task->pid, task->sid);

@@ -547,10 +551,6 @@ static int cr_set_creator(struct cr_ctx *ctx, struct task *task)
 			if (cr_placeholder_task(ctx, task) < 0)
 				return -1;
 		creator = session->phantom;
-	} else if (task->sid == parent->sid) {
-		/* (non-session-leader) inherit: creator is parent */
-		cr_dbg("pid %d: inherit sid %d\n", task->pid, task->sid);
-		creator = parent;
 	} else {
 		/* first make sure we know the session's creator */
 		if (!session->creator && session != cr_init_task(ctx)) {

@@ -643,6 +643,7 @@ static int cr_placeholder_task(struct cr_ctx *ctx, struct task *task)
 		session->children->prev_sib = holder;
 	}
 	session->children = holder;
+	session->phantom = holder;
 
 	/* reparent entry if necssary */
 	if (task->next_sib)

Yes, a process with ppid==1 and session id like the container-init can be viewed either an orphan task or as a child of containter-init.

This was the bug :(

It fixed the crash because it was a child of the session leader. If wouldn't work for an orphan with a different session id... (the fix above does).

You are definitely right in observing that a "phantom" task is not needed (although it doesn't break either) if an orphan has the same session id as the container root.

Oren.