Pádraig Brady <[email protected]> writes:
> I'd doc this an an improvement rather than a bug fix really.
> Also I can't see any practical cases this addresses.
> If a monitored command did PTRACE_TRACEME say to avoid debugging,
> then it would get stopped with any signal, so practially it
> would need to be a minimal tracer itself, thus timeout would not be involved.
> So really this is a robustness improvement for commands doing weird things.
Agreed, I wrote this on the OSS scanner bug tracker:
The man page for ptrace says "A process probably shouldn't make this
operation if its parent isn't expecting to trace it". So arguably,
timeout shouldn't need to consider this. Since handling it is only a
few extra lines of code that check the status of waitpid, I'm for
the change.
> The resume_command() name and return value contract confused me a little,
> especially as the PTRACE_DETACH result was ignored.
> In the attached I renamed the function to handle_ptrace_stop().
>
> The test has a race I think where timeout could send the SIGTERM
> before env has the ignore in place. So I've adjusted to using
> SIGURG which is ignored by default.
>
> Also the test could hang in the `grep TracerPid` check
> if a signal (say SIGWINCH from a resized terminal) caused
> the process to become stopped due to the preloaded PTRACE_TRACEME.
> It's better to protect all PTRACE_TRACEME uses with timeout.
Those changes look good to me.
> Also I think this test might ERROR on FreeBSD
> due to ptrace() being available but different.
> So I relaxed the ERROR to a SKIP when failing to build the shared lib.
I think I meant to do that. Originally, in src/timeout.c I just used
HAVE_PTRACE, but added the checks for __linux__ or __ANDROID__ after
seeing many other operating systems have a function of the same name
[1].
> The attached addresses all of the above points,
> for squashing in as you see fit.
I pushed it with those changes, and listed you as Co-author since you
ended up writing a sizable portion of the patch.
Also, I double-checked everything built on FreeBSD 16.0 and made sure
'make check' succeeded, just to be safe.
Thanks,
Collin
[1] https://en.wikipedia.org/wiki/Ptrace
