From: ebiederm@xmission.com (Eric W. Biederman)
To: Roland McGrath <roland@redhat.com>
Cc: Oleg Nesterov <oleg@redhat.com>, Andrew Morton <akpm@linux-foundation.org>,
       Pavel Emelyanov <xemul@openvz.org>,
       "Serge E. Hallyn" <serue@us.ibm.com>,
       Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>,
       linux-kernel@vger.kernel.org
References: <20081118175901.GA17134@redhat.com>
	<20081119185148.DC1D31544EB@magilla.localdomain>
Date: Wed, 19 Nov 2008 18:00:12 -0800
In-Reply-To: <20081119185148.DC1D31544EB@magilla.localdomain> (Roland
	McGrath's message of "Wed, 19 Nov 2008 10:51:48 -0800 (PST)")
Message-ID: <m1skpncg2b.fsf@frodo.ebiederm.org>
User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: [PATCH 1/2] protect /sbin/init from unwanted signals more
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2206
Lines: 55

Roland McGrath <roland@redhat.com> writes:

> The effect is fine, but that seems like a kludgey way to do it.
> I really don't think the sigaction case matters--certainly it will never
> come up with SIGKILL.  What about just this instead?
>
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -66,6 +66,15 @@ static int sig_ignored(struct task_struct *t, int sig)
>  		return 0;
>  
>  	handler = sig_handler(t, sig);
> +
> +	/*
> +	 * For init, short-circuit any signal without a handler.
> +	 * We won't allow them to be delivered, so don't even queue them.
> +	 */
> +	if (unlikely(signal->flags & SIGNAL_UNKILLABLE) &&
> +	    (handler == SIG_IGN || handler == SIG_DFL))
> +		return 1;
> +
>  	if (!sig_handler_ignored(handler, sig))
>  		return 0;
>  
> With that, I wonder if the SIGNAL_UNKILLABLE checks in get_signal_to_deliver
> and complete_signal are needed at all.  Hmm, I guess we do because this
> doesn't affect blocked signals, so they might be unblocked and delivered.
> (Note that since it doesn't affect blocked signals, this doesn't break init
> using sigwait if it wanted to.)

Ah.  That answers the question I had bouncing in the back of my head.

My original analysis of the situation was that we should not send blocked signals.
Treating handler != SIG_DFL as a permission check.  Not as an optimization.

Mostly because it is more consistent and uniform.

inits today don't do anything with blocked signals.  They explicitly ignore all signals,
they don't want to deal with an enable those they do.

Which I guess means in practice only SIGKILL and SIGSTOP are especially interesting,
and we can't block those so worrying about blocked signals is no big deal.

Which reminds me.  I need to retest, but I had a case where I had a trivial init
that set all signal handlers to SIG_IGN so it could ignore SIGCHLD.  And not
all of it's children were getting reaped automagically.  Do we have a bug in
the reparenting/reaping logic?

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/