Move parent fork dbgwait to syscallret() to avoid hang with RFSTOPPED with PT_FOLLOW_FORK.

[bdrewery]

Summary

Using fork1() with RFSTOPPED while being debugged with ptrace(PT_FOLLOW_FORK)
will cause the child to be forked off, never scheduled to attach debugger, and
the parent waiting for the child to attach at the end of do_fork() forever.

This should fix the hang with Linux fork(2) wrappers, which use RFSTOPPED, when
using ptrace(PT_FOLLOW_FORK). It also is much safer for other code that
may be using RFSTOPPED, such as vendor ABI modules, by removing the
possibility of the do_fork() code waiting forever for an unscheduled child.

I like the idea of the fork return hooks and still want to see that
implemented, but prefer this approach for this problem as otherwise the bug
is still present for other code to run into.

Test Plan

Tested with truss -f on native binaries, and Linux. Both work.

% truss -f /compat/linux/bin/bash -c "id & id &"
P78
% truss -f bash -c "id & id &"
...
% truss -f sh -c "id & id &"

The Kyua tests as of r288949 work as well.

/usr/tests/sys/kern # kyua test ptrace_test
ptrace_test:ptracefollow_fork_both_attached -> passed [0.010s]
ptrace_test:ptracefollow_fork_both_attached_unrelated_debugger -> passed [0.010s]
ptrace_test:ptracefollow_fork_child_detached -> passed [0.009s]
ptrace_test:ptracefollow_fork_child_detached_unrelated_debugger -> passed [0.010s]
ptrace_test:ptracefollow_fork_parent_detached -> passed [0.010s]
ptrace_test:ptracefollow_fork_parent_detached_unrelated_debugger -> passed [0.010s]
ptrace_test:ptracegetppid -> passed [0.009s]
ptrace_test:ptracenew_child_pl_syscall_code_fork -> passed [0.009s]
ptrace_test:ptracenew_child_pl_syscall_code_thread -> passed [0.009s]
ptrace_test:ptracenew_child_pl_syscall_code_vfork -> passed [0.009s]
ptrace_test:ptraceparent_sees_exit_after_child_debugger -> passed [0.015s]
ptrace_test:ptraceparent_sees_exit_after_unrelated_debugger -> passed [0.015s]
ptrace_test:ptraceparent_wait_after_attach -> passed [0.009s]
ptrace_test:ptraceparent_wait_after_trace_me -> passed [0.009s]

Results file id is usr_tests_sys_kern.20151006-222504-676851
Results saved to /root/.kyua/store/results.usr_tests_sys_kern.20151006-222504-676851.db

14/14 passed (0 failed)

[bdrewery]

I am guessing that the problem could be recreated from user land with rfork(2), without Linux code involvement. I am testing that idea now and will add a test case if so.

[bdrewery]

In D3829#78996, @bdrewery wrote:

I am guessing that the problem could be recreated from user land with rfork(2), without Linux code involvement. I am testing that idea now and will add a test case if so.

That was a silly thought :). Not possible of course to use RFSTOPPED from user land.

[jhb]

Userland cannot send RFSTOPPED via rfork(). It is an internal-only flag (part of the RFKERNEL_ONLY mask) and attempts to pass it via rfork() result in EINVAL.

[bdrewery]

In D3829#79027, @jhb wrote:

Userland cannot send RFSTOPPED via rfork(). It is an internal-only flag (part of the RFKERNEL_ONLY mask) and attempts to pass it via rfork() result in EINVAL.

Yeah I realized that and then found the code proving that. It would be stupid to expose it to user land.

[jhb]

I think this looks fine as far as implementing this design. The only open question is if we want to go this route to fix the issue or not. Looking at the code in linux_fork.c, it would actually be a bit of a PITA to pass in the required state to sv_fork_return(), especially in the linux_clone_proc() case. You would need to malloc a copy of the args and hand that off to the new thread so that sv_fork_return() could see it. Also, you would need to keep the parent "around" for linux_proc_init() (which needs threads from both the parent and child processes) or handle the race of it exiting before the child runs, etc. Leaving RFSTOPPED in place and moving the wait to syscallret() might actually be the simplest solution.