Re: detach() logic

[Denys V. <dvl...@re...>]

On 06/21/2013 02:07 AM, Dmitry V. Levin wrote:
>> However, now I see another, very simple bug in detach():
>>
>>                 sigstop_expected = (tcp->flags & TCB_IGNORE_ONE_SIGSTOP);
>>                 error = ptrace(PTRACE_DETACH, tcp->pid, 0, 0);
>>
>> What if sigstop_expected == 1 (IOW: TCB_IGNORE_ONE_SIGSTOP is set)?
>>
>> We will DETACH _before_ we eat and discard SIGSTOP.
>> After DETACH, we will do waitpid loop, see SIGSTOP,
>> and... try DETACH again! lol :(
> 
> No, in that case waitpid will fail with ECHILD, so there would be no try
> for the second PTRACE_DETACH.  With your recent commit v4.8-16-gfdfa47a,
> strace will complain:
> strace: detach: waitpid(12345): No child processes

Yes, I was wrong about how it will look to the user...

> I've actually managed to reproduce this warning with a very artificial test.
> 
>> Does it look like a real bug to you too?
> 
> It looks like a bug, SIGSTOP is certainly not expected after successful
> PTRACE_DETACH.

I contemplate the following fix:

	sigstop_expected = (tcp->flags & TCB_IGNORE_ONE_SIGSTOP);
	if (!sigstop_expected)   // <-- added this one line
		error = ptrace(PTRACE_DETACH, tcp->pid, 0, 0);

Do you see any problem with this approach?