linux-kernel - Re: [PATCH 5/6][v5] Protect cinit from blocked fatal signals

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 5 Jan 2009 16:16:08 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
Cc:	ebiederm@...ssion.com, roland@...hat.com, bastian@...di.eu.org,
	daniel@...ac.com, xemul@...nvz.org, containers@...ts.osdl.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/6][v5] Protect cinit from blocked fatal signals

On 12/27, Sukadev Bhattiprolu wrote:
>
> +		 * Container-init gets no signals it doesn't want from same
> +		 * container.
>  		 */
> -		if (unlikely(signal->flags & SIGNAL_UNKILLABLE) &&
> -		    !signal_group_exit(signal))
> +		if ((signal->flags & SIGNAL_UNKILLABLE) &&
> +				!sig_kernel_only(signr))

minor, but why did you remove unlikely() ?

And could you improve the changelog a bit? Please explain
that !signal_group_exit() was needed to handle the case
when the mt init does exit/exec and sends SIGKILL to all
sub-threads. Now this is covered by sig_kernel_only().

In short, please explain that if SIGNAL_UNKILLABLE task
sees the sig_kernel_only() signal here, then it was either
sent by the parent namespace or it was generated internally.
In both cases we should not drop it.

But your current changelog mostly explains why do we need
another SIGNAL_UNKILLABLE check in get_signal_to_deliver(),
this is orthogonal to the change itself.

Oleg.

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