linux-kernel - Re: PATCH? fix SIGNAL_STOP

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: <20070901114126.GA243@tv-sign.ru>
Date:	Sat, 1 Sep 2007 15:41:26 +0400
From:	Oleg Nesterov <oleg@...sign.ru>
To:	Roland McGrath <roland@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: PATCH? fix SIGNAL_STOP_DEQUEUED vs SIGCONT race

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.

-
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/