Re: [path][rfc] add PR_DETACH prctl command
From: Oleg Nesterov
Date: Mon Apr 04 2011 - 12:04:19 EST
Hi Stas,
On 04/04, Stas Sergeev wrote:
>
> Here's the patch that addresses your concerns
> about the late deleting from list.
> Also, the patch is shrunk twice.
> I think it is about to be trivial this time.
But it is very wrong at first glance...
> I still haven't solved the problems with checking
> parent and checking ptrace, so ignore them for
> now (or give me the hints:)
Not sure I understand your question...
But, once again. You should not use ptrace_reparented(). Reparanted or
not, the ptraced thread must not change its ->parent.
Sorry Stas, I have no time to read the patch, just one thing...
> + me->detach_code = arg2 << 8;
> ...
> + if (notif != DEATH_REAP)
> + me->detaching = 1;
> + else
> + list_move_tail(&me->sibling,
> + &me->real_parent->children);
This is simply wrong unless the caller is the group_leader. Probably
there was some confusion, I thought we already discussed this. But this
is minor...
> + while_each_thread(me, p) {
> + if (!ptrace_reparented(p))
> + p->parent = pid_ns->child_reaper;
> + p->real_parent = pid_ns->child_reaper;
Eek. Even ignoring ptrace, this is weird. We change parent/real_parent,
but we do not do list_move_tail(sibling) until wait_task_detached() !
No, I think we should not do this even if this was correct. I'll try
to nack this in any case, even if there were no immediate problems ;)
IMHO, this is insane.
But this is wrong. Well. Suppose that the caller of PR_DETACH exits
before the old parent does do_wait(). What /sbin/init (who is the new
parent) can do after it gets SIGCHLD? If can't see this zombie. Nor
the old parent can release this task due to ->detaching. Eventually
/sbin/init can reap it if it does, say, waitpid(-1), but still this
is wrong.
Or. Suppose that the old parent exits after its child does PR_DETACH.
You changed forget_original_parent() but this is not enough, note that
find_new_reaper() can pick the live sub-thread. In this case the child
will be moved to init's ->children list, and after that we are changing
->real_parent back.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/