lkml.org 
[lkml]   [2010]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
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    [from the cache]
©2003-2011 Jasper Spaans