On Mon, Mar 19, 2018 at 12:27:10PM +0100, Martin Pieuchot wrote:
> Thanks for the report.
>
> On 19/03/18(Mon) 09:49, Theo Buehler wrote:
> > This is a regression that came with the TOCTOU race fix in kern_sig.c 1.216:
> > https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/kern/kern_sig.c#rev1.216
> > [...]
> > Now gdb just hangs there and does nothing instead of exiting as
> > expected. It doesn't react to ^C but one can easily kill it with
> > ^Z and then kill %%.
>
> What happens is that the programs stays stopped. Or to be more precise
> re-enter the SSTOP'd state after ptrace(PT_KILL...) has been issued by
> gdb(1).
> The problem comes from the fact that CURSIG() is now called twice in
> userret(). That means that issignal() is also called twice. The fix
> is to treat SIGKILL as special if the process is currently traced.
>
> That's also what NetBSD is doing, so I synced our comment with their,
> without a typo.
>
> ok?
Works for me and makes sense. Thanks for the explanation and the quick fix.
ok (fwiw)
>
> Index: kern/kern_sig.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_sig.c,v
> retrieving revision 1.216
> diff -u -p -r1.216 kern_sig.c
> --- kern/kern_sig.c 26 Feb 2018 13:33:25 -0000 1.216
> +++ kern/kern_sig.c 19 Mar 2018 11:25:34 -0000
> @@ -1167,11 +1167,13 @@ issignal(struct proc *p)
> (pr->ps_flags & PS_TRACED) == 0)
> continue;
>
> - if ((pr->ps_flags & (PS_TRACED | PS_PPWAIT)) == PS_TRACED) {
> - /*
> - * If traced, always stop, and stay
> - * stopped until released by the debugger.
> - */
> + /*
> + * If traced, always stop, and stay stopped until released
> + * by the debugger. If our parent process is waiting for
> + * us, don't hang as we could deadlock.
> + */
> + if (((pr->ps_flags & (PS_TRACED | PS_PPWAIT)) == PS_TRACED) &&
> + signum != SIGKILL) {
> p->p_xstat = signum;
>
> if (dolock)
>
