Re: PATCH? fix SIGNAL_STOP_DEQUEUED vs SIGCONT race

[Oleg Nesterov <oleg@tv-sign.ru>]

On 09/01, Roland McGrath wrote:
>
> > However, this changes the behaviour when the task is ptraced. If the debugger
> > doesn't clear ->exit_code, SIGSTOP always succeeds after ptrace_stop(), even
> > if SIGCONT was sent in between. I can't decide whether this change is good
> > or bad, hopefully Roland can clarify.
> 
> Hmm.  I think this is bad.  
> 
> First, considering only the single-threaded case, there are debugger vs
> SIGCONT races.  Someone does kill(pid,SIGSTOP);kill(pid,SIGCONT); while pid
> is debugged.  The mandate for end user behavior here is that pid cannot
> wind up sitting in job control stop in the end.  Say the debugger is
> e.g. strace, simply printing every signal and passing it through.
> So say it goes:
> 	T			K			D
> 	merrily running ...				blocked in wait4
> 				kill(K, SIGSTOP)
> 	dequeue SIGSTOP
> 	 -> ptrace_stop
>  							wait4 -> K,{SIGSTOP}
> 				kill(K, SIGCONT)
> 							PTRACE_CONT,K,SIGSTOP
> 	do_signal_stop(SIGSTOP)
>  							wait4 -> K,{SIGSTOP}

Thanks Roland.

Yes, that is what I was worrying about.

> It's still probably a worthwhile cleanup to have the logic only in
> get_signal_to_deliver, and to fix the problem you cited.  It will take only
> a little extra code to handle the ptrace case too, i.e.
> 	if (sig_kernel_stop(signr) &&
>  	    current->sighand->action[signr-1] == SIG_DFL &&
>  	    !(current->signal->flags & SIGNAL_GROUP_EXIT))
> 		current->signal->flags |= SIGNAL_STOP_DEQUEUED;
> 	ptrace_stop(signr, signr, info);
> 	if (sig_kernel_stop(signr) && current->exit_code == signr &&
> 	    !(current->signal->flags & SIGNAL_STOP_DEQUEUED) &&
>  	    current->sighand->action[signr-1] == SIG_DFL)
> 		current->exit_code = 0;

Yes, I also thought about something like this, but tried to avoid because
it adds some complications. OTOH, this is not the fast path.

I'll try to think a bit more about this, and update the patch according
to your comments. Looks like we don't need to check SIGNAL_GROUP_EXIT
here, we are doing this later anyway.

Thanks!

Oleg.