linux-kernel - Re: [RFC][PATCH 5/6][v3] Protect cinit from unblocked SIG

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081222224618.GB1536@redhat.com>
Date:	Mon, 22 Dec 2008 23:46:18 +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, sukadev@...ibm.com
Subject: Re: [RFC][PATCH 5/6][v3] Protect cinit from unblocked SIG_DFL
	signals

On 12/20, Sukadev Bhattiprolu wrote:
>
> +static int sig_task_unkillable(struct task_struct *t, int same_ns)
> +{
> +	int flags = t->signal->flags;
> +
> +	if (unlikely(flags & SIGNAL_UNKILLABLE) ||
> +			(same_ns && (flags & SIGNAL_UNKILLABLE_FROM_NS)))
> +		return 1;

Hmm. I do not understand the point of the new flag,
SIGNAL_UNKILLABLE_FROM_NS (patch 3/6).

Actually, "same_ns" is a bad name, imho. It actually means "not from
parent ns", and this is not the same as "from the same ns".

Let's suppose we rename it, then the code becomes

	if (unlikely(flags & SIGNAL_UNKILLABLE) ||
		(!parent_ns && (flags & SIGNAL_UNKILLABLE_FROM_NS)))

But, parent_ns == T is not possible for the global init, so why
do we need the extra flag? we can just do

	if (unlikely(flags & SIGNAL_UNKILLABLE) && !parent_ns)
		return 1;

No?

> @@ -867,11 +886,17 @@ static int send_signal(int sig, struct siginfo *info, struct task_struct *t,
>  {
>  	struct sigpending *pending;
>  	struct sigqueue *q;
> +	int same_ns;
>  
>  	trace_sched_signal_send(sig, t);
>  
>  	assert_spin_locked(&t->sighand->siglock);
> -	if (!prepare_signal(sig, t))
> +
> +	same_ns = 1;
> +	if (siginfo_from_ancestor_ns(t, info))
> +		same_ns = 0;

This looks a bit strang, why not

	same_ns = siginfo_from_ancestor_ns(t, info);

?

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/