linux-kernel - Re: [PATCH v3 20/20] signal: Don't restart fork when signals come in.

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: <20180726134143.GB32718@redhat.com>
Date:   Thu, 26 Jul 2018 15:41:43 +0200
From:   Oleg Nesterov <oleg@...hat.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Wen Yang <wen.yang99@....com.cn>, majiang <ma.jiang@....com.cn>
Subject: Re: [PATCH v3 20/20] signal: Don't restart fork when signals come in.

On 07/24, Eric W. Biederman wrote:
>
> @@ -1979,6 +1983,8 @@ static __latent_entropy struct task_struct *copy_process(
>  			attach_pid(p, PIDTYPE_TGID);
>  			attach_pid(p, PIDTYPE_PGID);
>  			attach_pid(p, PIDTYPE_SID);
> +			p->signal->shared_pending.signal = delayed.signal;

Again, in this case we do not hold p->sighand->siglock (unless CLONE_SIGHAND),
so this should be done before list_add_tail/attach_pid above. Plus we need some
sort of barrier.

Or we can do

	if (!CLONE_SIGHAND)
		spin_lock_nested(child->siglock);

at the start of "if (likely(p->pid))" block.

> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1123,6 +1123,15 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
>  out_set:
>  	signalfd_notify(t, sig);
>  	sigaddset(&pending->signal, sig);
> +
> +	/* Let multiprocess signals appear after on-going forks */
> +	if (type > PIDTYPE_TGID) {
> +		struct multiprocess_signals *delayed;
> +		hlist_for_each_entry(delayed, &t->signal->multiprocess, node) {
> +			sigaddset(&delayed->signal, sig);

This is not enough, I think...

Suppose you send SIGSTOP and then SIGCONT to some process group. The 1st SIGSTOP
will be queued correctly, but the 2nd SIGCONT won't flush the pending SIGSTOP, you
need to modify prepare_signal() too.

Oleg.