Re: Kernel threads

[Roland McGrath <roland@redhat.com>]

> Yes sure, this change shoud be tested in -mm tree (I'll send the patch
> on Sunday after some testing). The only (afaics) problem is that with
> this change a kernel thread must not do do_fork(CLONE_THREAD). 

To clarify, the danger here is that an exit_signal=-1 leader would
self-reap and leave behind live threads with dangling ->group_leader
pointers.  This danger doesn't exist for normal user group leaders with
parents ignoring SIGCHLD, because exit_signal is never set to -1 until
do_notify_parent, which is never called until the last thread in the group
dies (except when ptrace'd, but then do_notify_parent never resets
exit_signal at all).  Is that right?

> I think it should not, but currently this is technically
> possible. Perhaps it makes sense to add BUG_ON(CLONE_THREAD &&
> group_leader->exit_signal==-1) in copy_process().

It probably wouldn't hurt to make it:

	if (user_mode(regs))
		BUG_ON(current->group_leader->exit_signal == -1);
	else
		BUG_ON((clone_flags & (CLONE_THREAD|CLONE_UNTRACED))
		       != CLONE_UNTRACED);

> 	zap_other_threads:
> 
> 		if (t != p->group_leader)
> 			t->exit_signal = -1;
> 
> looks like another leftover to me, we already depend on the fact that
> all sub-threads have ->exit_signal == -1 (otherwise, for example, a
> thread group just can't exit properly).

Yes, I agree it looks superfluous.

> While we are talking about kernel threads, there is something I can't
> undestand. kthread/daemonize use sigprocmask(SIG_BLOCK) to protect
> against signals. This doesn't look right to me, because this doesn't
> prevent the signal delivery, this only blocks signal_wake_up(). Every
> "killall -33 khelper" means a "struct siginfo" leak.

It does prevent the delivery (signal_pending() never set), but not the queuing.

> Imho, the kernel thread shouldn't play with ->blocked at all. Instead
> it should set SIG_IGN for all handlers. If it really needs, say, SIGCHLD,
> it should call allow_signal() anyway. Do you see any problems with this
> approach?

That sounds reasonable to me generally.  However, if kernel threads ever
spawn user children, they may not want the self-reaping behavior of
ignoring SIGCHLD even if they never dequeue the signal (because they want
to call do_wait).  There might be other strange caveats like that I'm not
thinking of.  


Thanks,
Roland