lkml.org 
[lkml]   [2010]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: Bug 16061 - single stepping in a signal handler can cause the single step flag to get "stuck"
    sorry for noise, forgot to mention...

    On 06/02, Oleg Nesterov wrote:
    >
    > However, what I am thinking about is the more "clever" change (it
    > passed ptrace-tests).
    >
    > Do you think it can be correct? I am asking because I never understood
    > the TIF_SINGLESTEP/TIF_FORCED_TF interaction. But otoh, shouldn't
    > TIF_FORCED_TF + X86_EFLAGS_TF always imply TIF_SINGLESTEP? at least
    > in handle_signal().
    >
    > IOW, help! To me, the patch below is also cleanup. But only if you think
    > it can fly ;)

    and it is not clear to me if we should keep this code

    /*
    * Clear TF when entering the signal handler, but
    * notify any tracer that was single-stepping it.
    * The tracer may want to single-step inside the
    * handler too.
    */
    regs->flags &= ~X86_EFLAGS_TF;

    in handle_signal(). If TF was set by us, it was cleared by
    user_disable_single_step(). Otherwise, why should we clear it if
    the tracer did set_flags(X86_EFLAGS_TF) ?

    Oleg.

    > --- 34-rc1/arch/x86/kernel/signal.c~BZ16061_MAYBE_FIX 2010-06-02 21:11:07.000000000 +0200
    > +++ 34-rc1/arch/x86/kernel/signal.c 2010-06-02 21:11:48.000000000 +0200
    > @@ -682,6 +682,7 @@ static int
    > handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
    > sigset_t *oldset, struct pt_regs *regs)
    > {
    > + bool stepping;
    > int ret;
    >
    > /* Are we from a system call? */
    > @@ -706,13 +707,10 @@ handle_signal(unsigned long sig, siginfo
    > }
    > }
    >
    > - /*
    > - * If TF is set due to a debugger (TIF_FORCED_TF), clear the TF
    > - * flag so that register information in the sigcontext is correct.
    > - */
    > - if (unlikely(regs->flags & X86_EFLAGS_TF) &&
    > - likely(test_and_clear_thread_flag(TIF_FORCED_TF)))
    > - regs->flags &= ~X86_EFLAGS_TF;
    > + stepping = test_thread_flag(TIF_SINGLESTEP);
    > + if (stepping)
    > + // do this before setup_sigcontext()
    > + user_disable_single_step(current);
    >
    > ret = setup_rt_frame(sig, ka, info, oldset, regs);
    >
    > @@ -748,8 +746,7 @@ handle_signal(unsigned long sig, siginfo
    > recalc_sigpending();
    > spin_unlock_irq(&current->sighand->siglock);
    >
    > - tracehook_signal_handler(sig, info, ka, regs,
    > - test_thread_flag(TIF_SINGLESTEP));
    > + tracehook_signal_handler(sig, info, ka, regs, stepping);
    >
    > return 0;
    > }



    \
     
     \ /
      Last update: 2010-06-02 22:11    [W:0.024 / U:118.816 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site