Re: x86: do_debug && PTRACE_SINGLESTEP broken by 08d68323d1f0c34452e614263b212ca556dae47f

[Frederic Weisbecker <fweisbec@gmail.com>]

On Fri, Dec 18, 2009 at 03:10:42AM +0100, Oleg Nesterov wrote:
> On 12/17, Roland McGrath wrote:
> >
> > Comparing to the old (2.6.32) logic, I think it might be this (untested).
> > I also note this is the sole use of get_si_code, seems like it should
> > just be rolled in here.
> 
> Well, it is too late for me to even try to read this patch ;)
> 
> but...
> 
> > @@ -569,14 +568,15 @@ dotraplinkage void __kprobes do_debug(struct pt_regs *regs, long error_code)
> >  	 * We already checked v86 mode above, so we can check for kernel mode
> >  	 * by just checking the CPL of CS.
> >  	 */
> > +	dr6 = tsk->thread.debugreg6;
> 
> why? we have "tsk->thread.debugreg6 = dr6" above


Yeah.


 
> >  	if ((dr6 & DR_STEP) && !user_mode(regs)) {
> >  		tsk->thread.debugreg6 &= ~DR_STEP;
> >  		set_tsk_thread_flag(tsk, TIF_SINGLESTEP);
> >  		regs->flags &= ~X86_EFLAGS_TF;
> 
> this looks strange... we set TIF_SINGLESTEP but clear X86_EFLAGS_TF


Yep, I don't understand what happens here either. This logic
was there before the refactoring and the comment indicates we want
to ignore traps from kernel. Why do we set this flag in a random
thread?



> > +	} else if (dr6 & (DR_STEP | DR_TRAP_BITS)) {
> > +		send_sigtrap(tsk, regs, error_code, get_si_code(dr6));
> >  	}
> > -	si_code = get_si_code(tsk->thread.debugreg6);
> > -	if (tsk->thread.debugreg6 & (DR_STEP | DR_TRAP_BITS))
> > -		send_sigtrap(tsk, regs, error_code, si_code);
> > +
> 
> can't understand how this change can fix the problem. We should always
> send SIGTRAP if the task returns to user-mode with X86_EFLAGS_TF?
> 
> OK. I blindly applied this patch, step-simple still fails.


Yep, that doesn't fix your problem but this patch makes sense
in that if we were not in user mode while the step occured,
we shouldn't send the signal.