On Mon, Oct 26, 2020 at 09:14:13AM -0700, Kyle Huey wrote: > > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c > > index 3c70fb34028b..0e7641ac19a8 100644 > > --- a/arch/x86/kernel/traps.c > > +++ b/arch/x86/kernel/traps.c > > @@ -799,6 +799,13 @@ static __always_inline unsigned long > > debug_read_clear_dr6(void) > > */ > > current->thread.virtual_dr6 = 0; > > > > + /* > > + * If PTRACE requested SINGLE(BLOCK)STEP, make sure to reflect that > > in > > + * the ptrace visible DR6 copy. > > + */ > > + if (test_thread_flag(TIF_BLOCKSTEP) || > > test_thread_flag(TIF_SINGLESTEP)) > > + current->thread.virtual_dr6 |= dr6 & DR_STEP; > > + > > /* > > * The SDM says "The processor clears the BTF flag when it > > * generates a debug exception." Clear TIF_BLOCKSTEP to keep > > I don't think the `& DR_STEP` should be necessary, that bit should be > set by the hardware, I believe.
Yeah, the idea is to only 'copy' the DR_STEP bit, not any others. > Also if a program sets TF on itself in EFLAGS and generates a trap it > should still have DR_STEP set in the ptrace-visible dr6, which this > won't do. Why? The state was not requested by ptrace(). And the signal should have it's si_code set to TRACE_TRACE. > Is there a reason not to always copy dr6 into virtual_dr6 here, > regardless of TIF_SINGLESTEP/etc? Why should we expose DR6 bits that are the result of kernel internal actions? Suppose someone sets an in-kernel breakpoint (perf can do that for example), the #DB happens and we write whatever into virtual_dr6. Even if you have a userspace breakpoint not through ptrace(), then why should we expose that in dr6 ? In that respect, I think the current virtual_dr6 = 0 is placed wrong, it should only be in exc_debug_user(). The only 'problem' then is that we seem to be able to loose BTF, but perhaps that is already an extant bug. Consider: - perf: setup in-kernel #DB - tracer: ptrace(PTRACE_SINGLEBLOCK) - tracee: #DB on perf breakpoint, looses BTF - tracee .. never triggers actual blockstep Hmm ? Should we re-set BTF when TIF_BLOCKSTEP && !user_mode(regs) ?
