Always clear TDB_USERWR before fetching system call arguments.

[jhb]

Summary

Always clear TDB_USERWR before fetching system call arguments,
The TDB_USERWR flag may still be set after a debugger detaches from a
process via PT_DETACH. Previously the flag would never be cleared forcing
a double fetch of the system call arguments for each system call. Note that
the flag cannot be cleared at PT_DETACH time in case one of the threads in the
process is currently stopped in syscallenter() and the debugger has modified
the arguments for that pending system call before detaching.

Test Plan

• Run a fork test under gdb while following the child of the fork. After gdb detaches from the parent it will log double ktrace entries for each system call due to the stucky TDB_USERWR flag without this change.

[jhb]

I initially tried clearing TDB_USERWR in syscall_enter() after it was
detected, but that still resulted in a double log of the first system
call after PT_DETACH.

Hmm, this version though means that a thread stuck in SCE when the detach
comes in might not re-read arguments changed by the tracer just before the
detach, so this is no good. Instead, we need to let TDB_USERWR persist even
after detach until the thread resumes.

I guess the "right" thing to do is to always clear TDB_USERWR in syscall_enter()
before fetching arguments. However, that seems unfortunate as it means adding
a PROC_LOCK() for all non-traced threads.

[jhb]

• Don't clear flags on detach. Clear TDB_USERWR if needed in syscall_enter().

[jhb]

sys/kern/subr_syscall.c

66

Setting traced to true here even if P_TRACED isn't set is a bit hackish, but all it enables is setting TDB_SCE here and then clearing it at the end of syscall_enter(). The upside is it avoids needing PROC_LOCK in the common case. Only the first syscall after a PT_DETACH that left TDB_USERWR set should be effected.

[kib]

sys/kern/subr_syscall.c

70

Perhaps

	if ((p->p_flag & P_TRACED) != 0)
		traced = 1;
	if (traced || (td->td_dbgflags & TDB_USERWR) != 0) {
		PROC_LOCK(p);
		td->td_dbgflags &= ~TDB_USERWR;
		if (traced)
			td->td_dbgflags |= TDB_SCE;
		PROC_UNLOCK(p);
	} else
		traced = 0;

[kib]

sys/kern/subr_syscall.c

70

Of course, there is bug. More reasonable version is below.

	traced = (p->p_flag & P_TRACED) != 0;
	if (traced || (td->td_dbgflags & TDB_USERWR) != 0) {
		PROC_LOCK(p);
		td->td_dbgflags &= ~TDB_USERWR;
		if (traced)
			td->td_dbgflags |= TDB_SCE;
		PROC_UNLOCK(p);
	}

[jhb]

• Only set TDB_SCE and set traced if the process is traced.