Messages in this thread | | | Date | Wed, 12 Oct 2011 21:59:35 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH v5 3.1.0-rc4-tip 26/26] uprobes: queue signals while thread is singlestepping. |
| |
On 10/11, Srikar Dronamraju wrote: > > --- a/kernel/uprobes.c > +++ b/kernel/uprobes.c > @@ -1366,6 +1366,26 @@ static bool sstep_complete(struct uprobe *uprobe, struct pt_regs *regs) > } > > /* > + * While we are handling breakpoint / singlestep, ensure that a > + * SIGTRAP is not delivered to the task. > + */ > +static void __clear_trap_flag(void) > +{ > + sigdelset(¤t->pending.signal, SIGTRAP); > + sigdelset(¤t->signal->shared_pending.signal, SIGTRAP); > +} > + > +static void clear_trap_flag(void) > +{ > + if (!test_and_clear_thread_flag(TIF_SIGPENDING)) > + return; > + > + spin_lock_irq(¤t->sighand->siglock); > + __clear_trap_flag(); > + spin_unlock_irq(¤t->sighand->siglock); > +}
And this is called before and after the step.
Confused... For what? What makes SIGTRAP special? Where does this signal come from? If you meant do_debug() this seems impossible, uprobe_exception_notify(DIE_DEBUG) returns NOTIFY_STOP.
I certainly missed something.
> @@ -1401,13 +1422,18 @@ void uprobe_notify_resume(struct pt_regs *regs) > if (!utask) > goto cleanup_ret; > } > - /* TODO Start queueing signals. */ > utask->active_uprobe = u; > handler_chain(u, regs); > utask->state = UTASK_SSTEP; > - if (!pre_ssout(u, regs, probept)) > + if (!pre_ssout(u, regs, probept)) { > + sigfillset(&masksigs); > + sigdelsetmask(&masksigs, > + sigmask(SIGKILL)|sigmask(SIGSTOP)); > + current->saved_sigmask = current->blocked; > + set_current_blocked(&masksigs);
OK, we already discussed the problems with this approach.
> + clear_trap_flag();
In any case unneeded, we already blocked SIGTRAP.
> @@ -1418,8 +1444,8 @@ void uprobe_notify_resume(struct pt_regs *regs) > utask->state = UTASK_RUNNING; > user_disable_single_step(current); > xol_free_insn_slot(current); > - > - /* TODO Stop queueing signals. */ > + clear_trap_flag();
This is what I can't understand.
> + set_restore_sigmask();
No, this is not right. If we have a pending signal, the signal handler will run with the almost-all-blocked mask we set before.
And this is overkill anyway, you could simply do set_current_blocked(¤t->saved_sigmask).
->saved_sigmask is only used when we return from syscall, so uprobes can (ab)use it safely.
> @@ -1433,7 +1459,7 @@ void uprobe_notify_resume(struct pt_regs *regs) > put_uprobe(u); > set_instruction_pointer(regs, probept); > } else > - /*TODO Return SIGTRAP signal */ > + send_sig(SIGTRAP, current, 0);
This change looks "offtopic" to the problems we are discussing.
Or I missed something and this is connected to the clear_trap_flag() somehow?
Oleg.
| |