linux-kernel - Re: [PATCH 1/3] powerpc: kprobes: add support for KPROBES_ON

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]

Date:   Fri, 17 Feb 2017 01:18:41 +0530
From:   "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To:     Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>
Cc:     linuxppc-dev@...ts.ozlabs.org,
        Masami Hiramatsu <mhiramat@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] powerpc: kprobes: add support for KPROBES_ON_FTRACE

On 2017/02/15 09:58AM, Ananth N Mavinakayanahalli wrote:
> On Wed, Feb 15, 2017 at 12:28:34AM +0530, Naveen N. Rao wrote:
> > Allow kprobes to be placed on ftrace _mcount() call sites. This
> > optimization avoids the use of a trap, by riding on ftrace
> > infrastructure.
> > 
> > This depends on HAVE_DYNAMIC_FTRACE_WITH_REGS which depends on
> > MPROFILE_KERNEL, which is only currently enabled on powerpc64le with
> > newer toolchains.
> > 
> > Based on the x86 code by Masami.
> > 
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@...ux.vnet.ibm.com>
> > ---
> >  arch/powerpc/Kconfig                 |   1 +
> >  arch/powerpc/include/asm/kprobes.h   |  10 ++++
> >  arch/powerpc/kernel/Makefile         |   3 ++
> >  arch/powerpc/kernel/kprobes-ftrace.c | 100 +++++++++++++++++++++++++++++++++++
> >  arch/powerpc/kernel/kprobes.c        |   4 +-
> >  arch/powerpc/kernel/optprobes.c      |   3 ++
> >  6 files changed, 120 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/powerpc/kernel/kprobes-ftrace.c
> 
> You'll also need to update
> Documentation/features/debug/kprobes-on-ftrace/arch-support.txt

Sure.

> 
> > +/* Ftrace callback handler for kprobes */
> > +void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
> > +			   struct ftrace_ops *ops, struct pt_regs *regs)
> > +{
> > +	struct kprobe *p;
> > +	struct kprobe_ctlblk *kcb;
> > +	unsigned long flags;
> > +
> > +	/* Disable irq for emulating a breakpoint and avoiding preempt */
> > +	local_irq_save(flags);
> > +	hard_irq_disable();
> > +
> > +	p = get_kprobe((kprobe_opcode_t *)nip);
> > +	if (unlikely(!p) || kprobe_disabled(p))
> > +		goto end;
> > +
> > +	kcb = get_kprobe_ctlblk();
> > +	if (kprobe_running()) {
> > +		kprobes_inc_nmissed_count(p);
> > +	} else {
> > +		unsigned long orig_nip = regs->nip;
> > +		/* Kprobe handler expects regs->nip = nip + 1 as breakpoint hit */
> 
> Can you clarify this? On powerpc, the regs->nip at the time of
> breakpoint hit points to the probed instruction, not the one after.

Ah -- great catch! As it turns out, we actually need to set this back by 
an instruction due to the way ftrace works. I'll make the change.


Thanks,
Naveen