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

From: Oleg Nesterov
Date: Thu Jul 26 2018 - 09:41:48 EST


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.