Messages in this thread Patch in this message | | | Date | Wed, 2 Jun 2010 22:07:20 +0200 | From | Oleg Nesterov <> | Subject | Re: 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(¤t->sighand->siglock); > > - tracehook_signal_handler(sig, info, ka, regs, > - test_thread_flag(TIF_SINGLESTEP)); > + tracehook_signal_handler(sig, info, ka, regs, stepping); > > return 0; > }
| |