linux-kernel - Re: [PATCH 12/X] uprobes: x86: introduce abort

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111021162631.GB2552@in.ibm.com>
Date:	Fri, 21 Oct 2011 21:56:31 +0530
From:	Ananth N Mavinakayanahalli <ananth@...ibm.com>
To:	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...e.hu>,
	Steven Rostedt <rostedt@...dmis.org>,
	Linux-mm <linux-mm@...ck.org>,
	Arnaldo Carvalho de Melo <acme@...radead.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Jonathan Corbet <corbet@....net>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Hugh Dickins <hughd@...gle.com>,
	Christoph Hellwig <hch@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andi Kleen <andi@...stfloor.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jim Keniston <jkenisto@...ux.vnet.ibm.com>,
	Roland McGrath <roland@...k.frob.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 12/X] uprobes: x86: introduce abort_xol()

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/