lkml.org 
[lkml]   [2011]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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(&current->pending.signal, SIGTRAP);
    > + sigdelset(&current->signal->shared_pending.signal, SIGTRAP);
    > +}
    > +
    > +static void clear_trap_flag(void)
    > +{
    > + if (!test_and_clear_thread_flag(TIF_SIGPENDING))
    > + return;
    > +
    > + spin_lock_irq(&current->sighand->siglock);
    > + __clear_trap_flag();
    > + spin_unlock_irq(&current->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(&current->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.



    \
     
     \ /
      Last update: 2011-10-12 22:07    [W:4.288 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site