• Oleg Nesterov's avatar
    ptrace/x86: Partly fix set_task_blockstep()->update_debugctlmsr() logic · 95cf00fa
    Oleg Nesterov authored
    Afaics the usage of update_debugctlmsr() and TIF_BLOCKSTEP in
    step.c was always very wrong.
    
    1. update_debugctlmsr() was simply unneeded. The child sleeps
       TASK_TRACED, __switch_to_xtra(next_p => child) should notice
       TIF_BLOCKSTEP and set/clear DEBUGCTLMSR_BTF after resume if
       needed.
    
    2. It is wrong. The state of DEBUGCTLMSR_BTF bit in CPU register
       should always match the state of current's TIF_BLOCKSTEP bit.
    
    3. Even get_debugctlmsr() + update_debugctlmsr() itself does not
       look right. Irq can change other bits in MSR_IA32_DEBUGCTLMSR
       register or the caller can be preempted in between.
    
    4. It is not safe to play with TIF_BLOCKSTEP if task != current.
       DEBUGCTLMSR_BTF and TIF_BLOCKSTEP should always match each
       other if the task is running. The tracee is stopped but it
       can be SIGKILL'ed right before set/clear_tsk_thread_flag().
    
    However, now that uprobes uses user_enable_single_step(current)
    we can't simply remove update_debugctlmsr(). So this patch adds
    the additional "task == current" check and disables irqs to avoid
    the race with interrupts/preemption.
    
    Unfortunately this patch doesn't solve the last problem, we need
    another fix. Probably we should teach ptrace_stop() to set/clear
    single/block stepping after resume.
    
    And afaics there is yet another problem: perf can play with
    MSR_IA32_DEBUGCTLMSR from nmi, this obviously means that even
    __switch_to_xtra() has problems.
    Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
    95cf00fa
step.c 5.93 KB