svn commit: r332454 - in head/sys: amd64/amd64 amd64/ia32 amd64/linux amd64/linux32 i386/i386

[John Baldwin]

On Thursday, April 12, 2018 08:43:39 PM Konstantin Belousov wrote:
> Author: kib
> Date: Thu Apr 12 20:43:39 2018
> New Revision: 332454
> URL: https://svnweb.freebsd.org/changeset/base/332454
> 
> Log:
>   Fix PSL_T inheritance on exec for x86.
>   
>   The miscellaneous x86 sysent->sv_setregs() implementations tried to
>   migrate PSL_T from the previous program to the new executed one, but
>   they evaluated regs->tf_eflags after the whole regs structure was
>   bzeroed.  Make this functional by saving PSL_T value before zeroing.
>   
>   Note that if the debugger is not attached, executing the first
>   instruction in the new program with PSL_T set results in SIGTRAP, and
>   since all intercepted signals are reset to default dispostion on
>   exec(2), this means that non-debugged process gets killed immediately
>   if PSL_T is inherited.  In particular, since suid images drop
>   P_TRACED, attempt to set PSL_T for execution of such program would
>   kill the process.
>   
>   Another issue with userspace PSL_T handling is that it is reset by
>   trap().  It is reasonable to clear PSL_T when entering SIGTRAP
>   handler, to allow the signal to be handled without recursion or
>   delivery of blocked fault.  But it is not reasonable to return back to
>   the normal flow with PSL_T cleared.  This is too late to change, I
>   think.

Hmm, I had wanted to write an explicit test for this in ptrace_test.c so that
we could ensure other architectures that support hardware stepping (like
aarch64) use matching semantics.  It wasn't clear to me if clearing PSL_T on
exec() isn't actually more correct.  Exec will report a PL_FLAG_EXEC ptrace
stop and the debugger can PT_STEP from that stop if it wants to continue
stepping post-exec.

The trap() case is indeed interesting, but I think the concern you raise is
largely mitigated by having the debugger simple re-enable stepping after
resuming from the event reported from trapsignal().  OTOH, we explicitly
clear PSL_T in sendsig() so that signal handlers don't step.  I feel like we
probably should not do this for traced processes as this fix single stepping
in a debugger to properly report a step for the start of a signal handler.
FWIW, Linux single steps into signal handlers instead of over them as we
currently do.  (I have even some thoughts on how to fix stepping for
architectures like MIPS that do software stepping by letting the debugger
enable a "report a step for first signal instruction" mode while it is
stepping a thread and then reporting an explicit SIGTRAP ptracestop()
during sendsig.)

-- 
John Baldwin