Re: [PATCH 12/X] uprobes: x86: introduce abort_xol()

[Ananth N Mavinakayanahalli]

On Fri, Oct 21, 2011 at 08:12:07PM +0530, Srikar Dronamraju wrote:

...

> > If it is not clear, abort_xol() is needed when we should
> > re-execute the original insn (replaced with int3), see the
> > next patch.
> 
> We should be removing the breakpoint in abort_xol().
> Otherwise if we just set the instruction pointer to int3 and signal a
> sigill, then the user may be confused why a breakpoint is generating
> SIGILL.
> 
> > ---
> >  arch/x86/include/asm/uprobes.h |    1 +
> >  arch/x86/kernel/uprobes.c      |    9 +++++++++
> >  2 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/uprobes.h b/arch/x86/include/asm/uprobes.h
> > index f0fbdab..6209da1 100644
> > --- a/arch/x86/include/asm/uprobes.h
> > +++ b/arch/x86/include/asm/uprobes.h
> > @@ -51,6 +51,7 @@ extern void set_instruction_pointer(struct pt_regs *regs, unsigned long vaddr);
> >  extern int pre_xol(struct uprobe *uprobe, struct pt_regs *regs);
> >  extern int post_xol(struct uprobe *uprobe, struct pt_regs *regs);
> >  extern bool xol_was_trapped(struct task_struct *tsk);
> > +extern void abort_xol(struct pt_regs *regs);
> >  extern int uprobe_exception_notify(struct notifier_block *self,
> >  				       unsigned long val, void *data);
> >  #endif	/* _ASM_UPROBES_H */
> > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> > index c861c27..bc11a89 100644
> > --- a/arch/x86/kernel/uprobes.c
> > +++ b/arch/x86/kernel/uprobes.c
> > @@ -511,6 +511,15 @@ bool xol_was_trapped(struct task_struct *tsk)
> >  	return false;
> >  }
> > 
> > +void abort_xol(struct pt_regs *regs)
> > +{
> > +	// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
> > +	// !!! Dear Srikar and Ananth, please implement me !!!
> > +	// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
> > +	struct uprobe_task *utask = current->utask;
> > +	regs->ip = utask->vaddr;
> 
> nit:
> Shouldnt we be setting the ip to the next instruction after this
> instruction?

No, since we should re-execute the original instruction after removing
the breakpoint.

Also, wrt ip being set to the next instruction on a breakpoint hit,
that's arch specific. For instance, on x86, it points to the next
instruction, while on powerpc, the nip points to the breakpoint vaddr
at the time of exception.

Ananth

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/