Re: [PATCH 1/2] kill_something_info: misc cleanups

[Oleg Nesterov <oleg@tv-sign.ru>]

On 12/18, Eric W. Biederman wrote:
>
> Oleg Nesterov <oleg@tv-sign.ru> writes:
> 
> > Btw, de_thread() already takes care about multithread init, but
> > get_signal_to_deliver() does not:
> >
> > 	if (current == child_reaper(current))
> > 		continue;
> 
> Probably just: current->group_leader == child_reaper(current).

No. deadlock on exec.

> Can we do the ignore in send_signal?

No. It is not possible immediately with the current implemantation,
but the main reason is that we want to retain the "init is protected
from all signals it doesn't explicitly setup a signal handler for"
property. Consider init doing

	signal(SIGTERM, handler);
	...
	signal(SIGTERM, SIG_DFL);	<----- SIGTERM comes

so we should do something for init on receive-signal path. Yes, yes,
we _can_ solve this in other way (say, change sys_rt_sigaction), but
this is nasty and doesn't solve other problems.

> > This doesn't protect init from SIGKILL if we send it to sub-thread (and
> > this can happen even if we use kill(1, sig), not tkill). Yes, the main
> > thread will survive, but still this is not what we want. SIGSTOP will
> > manage to stop entire group because sub-thread sets ->group_stop_count.
> 
> Yep.  We need to fix this.  It doesn't happen today because we don't
> have a multi-threaded init.  But as soon as untrusted users can have
> their own init this becomes we need to handle everything properly.

This is a longstanding problem, an it is connected to other longstanding
problems (say, fatal signal may be lost on exec). I wish I had a time to
at least try to find a solution.

Probably I'll find some time at the beginning of January, I have some
vague ideas. Hmm... at least I had :)

> We need two specific helpers.
> 1) To detect the real machine init and the special things we have to
>    do for it.
> 2) To detect the init of a pid namespace for user visible semantic
>    purposes like signal handling (is there anything else).

Yes, I understand. My intent was to push this cleanup before multiple
pid namespaces will change kill_something_inf(). But as I said it is
minor, and since you and Christoph don't like it (for whatever reason)
please forget it.

Oleg.