Re: [PATCHv11 2.6.36-rc2-tip 5/15] 5: uprobes: Uprobes (un)registration and exception handling.

[Peter Zijlstra <peterz@infradead.org>]

On Wed, 2010-09-01 at 23:43 +0200, Peter Zijlstra wrote:
> > +void uprobe_notify_resume(struct pt_regs *regs)
> > +{
> > +     struct uprobe_process *uproc;
> > +     struct uprobe_probept *ppt;
> > +     struct uprobe_task *utask;
> > +     struct uprobe *u;
> > +     unsigned long probept;
> > +
> > +     utask = current->utask;
> > +     uproc = current->mm->uproc;
> > +     if (unlikely(!utask)) {
> > +             utask = add_utask(uproc);
> > +
> > +             /* Failed to allocate utask for the current task. */
> > +             BUG_ON(!utask);
> > +             probept = uprobes_get_bkpt_addr(regs);
> > +             ppt = find_probept(uproc, probept);
> > +
> > +             /*
> > +              * The probept was refcounted in uprobe_bkpt_notifier;
> > +              * Hence it would be mysterious to miss ppt now
> > +              */
> > +             WARN_ON(!ppt);
> > +             utask->active_ppt = ppt;
> > +             utask->state = UTASK_BP_HIT;
> > +     } else
> > +             ppt = utask->active_ppt;
> 
> You can replace this with:
> 
>  addr = instruction_pointer(task_pt_regs(current)) -
> ip_advancement_by_brkpt_insn;
> 
> and then proceed from there like described below to obtain the struct
> uprobe.
> 
> You can infer the SS/HIT state by checking if the user-addr is in the
> XOL area or not.

Right, so one problem I overlooked is that you need to have the actual
probe to compute the jump address, but that could be fixed by
implementing boosted probes.

> > +     if (utask->state == UTASK_BP_HIT) {
> > +             utask->state = UTASK_SSTEP;
> > +             u = ppt->uprobe;
> > +             if (u && u->handler)
> > +                     u->handler(u, regs);
> > +
> > +             if (!pre_ssout(ppt, regs))
> > +                     arch_uprobe_enable_sstep(regs);
> > +     } else if (utask->state == UTASK_SSTEP) {
> > +             if (sstep_complete(regs, ppt)) {
> > +                     put_probept(ppt);
> > +                     utask->active_ppt = NULL;
> > +                     utask->state = UTASK_RUNNING;
> > +                     arch_uprobe_disable_sstep(regs);
> > +             }
> > +     }
> > +}
> > +
> > +/*
> > + * uprobe_bkpt_notifier gets called from interrupt context
> > + * it gets a reference to the ppt and sets TIF_UPROBE flag,
> > + */
> > +int uprobe_bkpt_notifier(struct pt_regs *regs)
> > +{
> > +     struct uprobe_process *uproc;
> > +     struct uprobe_probept *ppt;
> > +     struct uprobe_task *utask;
> > +     unsigned long probept;
> > +
> > +     if (!current->mm || !current->mm->uproc)
> > +             /* task is currently not uprobed */
> > +             return 0;
> > +
> > +     uproc = current->mm->uproc;
> > +     utask = current->utask;
> > +     probept = uprobes_get_bkpt_addr(regs);
> 
> Its an address, not a struct uprobe_probept * so call it addr if
> anything.
> 
> > +     ppt = find_probept(uproc, probept);
> > +     if (!ppt)
> > +             return 0;
> > +     get_probept(ppt);
> > +     if (utask) {
> > +             utask->active_ppt = ppt;
> > +             utask->state = UTASK_BP_HIT;
> > +     }
> 
> Right, so all this should die..
> 
> Obtain the address, lookup the vma, get the file, compute the offset
> inside that file -> {inode,offset}

Except we need to stabilize the vma tree to do the lookup and that
currently requires mmap_sem, I guess until we get Nick's per-pte
vma-tree we could fudge that by adding a spinlock around the rb_tree
modifications -- I've got a patch for that somewhere I think.

> Do the lookup in the global tree to obtain the uprobe, and possibly
> execute the probe handler right here in interrupt context, if it fails
> with -EFAULT, continue with the below:
> 
> > +#ifdef CONFIG_ARCH_SUPPORTS_UPROBES
> > +     set_thread_flag(TIF_UPROBE);
> > +#endif
> 
> How can we ever get here if the architecture doesn't support uprobes?
> 
> > +     return 1;
> > +}