Don't reap orphan processes during wait.

[jhb]

Summary

Do not allow a process to reap an orphan (a child currently being
traced by another process such as a debugger). The parent process
does need to check for matching orphan pids to avoid returning
ECHILD if an orphan has exited, but it should not return the exited
status for the child until after the debugger has detached from the
orphan process either explicitly or implicitly via wait().

Add a test for for this case.

Another note: I found it a bit odd that KERN_PROC_PID fails for zombies
rather than returning a valid kinfo_proc with ki_stat == SZOMB. All
the other KERN_PROC_* sysctl handlers will return zombies, but not
KERN_PROC_PID.

Test Plan

• The new test case fails without this fix and passes with it.

[kib]

sys/kern/kern_exit.c

1286 ↗

(On Diff #5681)

Isn't passing non-NULL wrusage and siginfo is a waste for check_only case ?

Otherwise, the patch looks fine.

[jhb]

sys/kern/kern_exit.c

1286 ↗

(On Diff #5681)

Actually, it is. Also, now that I think about it, I can change this to be simpler by forcing
WNOWAIT to be present and treating and non-zero value as bumping nfound++. I originally wanted to do that but was hung up on that harvesting rusage, etc. Passing a NULL rusage will work fine though, and I think this is overall cleaner than adding another arg to proc_to_reap().

[jhb]

• Simplify orphan check by using WNOWAIT and NULL parameters.
• Revert the WNOAIT bit.
• Add another test to check for incorrect ECHILD errors when the only children are orphans.

[ngie]

head/tests/sys/kern/ptrace_test.c

208–224

Why can't this use waitpid(2) with 0?

227–229

This comment's a bit misleading... from waitpid(2):

awaited, -1 is returned with errno set to ECHILD.  Otherwise, if WNOHANG
is specified and there are no stopped, continued or exited children, 0 is
returned.  If an error is detected or a caught signal aborts the call, a

It doesn't say the PID returned is empty..

270–271

I don't think using ATF_REQUIRE in child processes is a good idea. I have no idea if the processes and file descriptors are shared between the processes. Please use assert+_exit

371

Should this be 0 or 1?

[jhb]

head/tests/sys/kern/ptrace_test.c

208–224

What do you mean? To check for the old bug, I need to wait for the process to be a zombie. I can't use wait to do that as then it will wait forever in a "fixed" implementation (since the parent process wouldn't return from wait until the debugger has seen the exit). I am specifically trying to arrange things so that the child is a zombie and then confirming that the parent will fail to wait in that case. Only after confirming that do I release the debugger to do its wait() of the child at which point a wait() in the parent should harvest the child.

227–229

I don't think that's that much of a stretch to see that "empty pid" == "pid of 0". Would you say that a non-blocking read on a socket with no data would return an empty buffer when it returns a size of zero? This is similar to that. Changing it to "pid of 0" is fine though.

270–271

It's fork(), of course descriptors are shared. Looking at ATF_REQUIRE's implementation, it just calls exit(). I can add a 'ATF_CHILD_REQUIRE()' macro though that is a child-safe variant (basically reimplementing it all by hand which sucks). assert() wouldn't be good because it would leave cores lying around.

371

The index into dpipe[] or the return value? (The question is ambiguous).

This is relying on the debugger implicitly closing dpipe[0] when it exits to trigger an EOF read on dpipe[1] in the parent. The debugger has already closed it's copy of dpipe[1], so any read on dpipe[0] would return EOF before the debugger exits. Similarly, the parent just closed dpipe[0] so it can't read from it without getting an instant EBADF. The only thing that can be read from is dpipe[1].

The return value needs to be 0 since we are waiting for EOF from the implicit close on exit().