Re: Q: user_enable_single_step() && update_debugctlmsr()

[Sebastian Andrzej Siewior]

On 08/01/2012 05:14 PM, Oleg Nesterov wrote:
> On 08/01, Sebastian Andrzej Siewior wrote:
> > On 08/01/2012 05:01 PM, Oleg Nesterov wrote:
> > > On 08/01, Sebastian Andrzej Siewior wrote:
> > > > So a patch like
> > > > --- a/arch/x86/kernel/step.c
> > > > +++ b/arch/x86/kernel/step.c
> > > > @@ -173,8 +173,8 @@ static void enable_step(struct task_struct *child,
> > > > bool block)
> > > >                   unsigned long debugctl = get_debugctlmsr();
> > > >
> > > >                   debugctl |= DEBUGCTLMSR_BTF;
> > > > -               update_debugctlmsr(debugctl);
> > > >                   set_tsk_thread_flag(child, TIF_BLOCKSTEP);
> > > > +               update_debugctlmsr(debugctl);
> > > >           } else if (test_tsk_thread_flag(child, TIF_BLOCKSTEP)) {
> > > >                   unsigned long debugctl = get_debugctlmsr();
> > > >
> > > > should fix the race
> > >
> > > No, I don't think it can fix something ;) or make any difference.
> >
> > Why? You _first_ set the task flag
>
> Yes, and this task is "child".
>
> > followed by the CPU register. Now
> > switch_to() would see the bit set and act.
>
> child sleeps and doesn't participate in switch_to(). Debugger and another
> (unrelated) task do.

This is confusing.

In order to allow the debugger to ptrace()->enable_blockstep() the
child has to be stopped/traced. We switch X86_EFLAGS_TF in child's regs
and enable DEBUGCTLMSR_BTF for the debugger which is wrong. If we quit
to userspace then the CPU on which the debugger runs has
DEBUGCTLMSR_BTF. If the tracee task runs on the same then nothing 
happens, the bit remains set.
If the tracee happens to run on a different CPU then switch_to() will
enable the DEBUGCTLMSR_BTF bit for the debugger's CPU and switch_to() 
will enable it also on the other CPU.
I added a few printks in the source and I see output that
__switch_to_xtra() enables the bit as well as in enable_single() for
debugger's CPU. I didn't find where the single step is disabled for the
tracee. I haven't notice this in __switch_to_xtra() nor or in
disable_single_step().

> Oleg.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/