All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: "Jon Medhurst (Tixy)" <tixy@linaro.org>
Cc: Russell King <linux@armlinux.org.uk>,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>,
	Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H . Peter Anvin" <hpa@zytor.com>, Wang Nan <wangnan0@huawei.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	"David A . Long" <dave.long@linaro.org>,
	Sandeepa Prabhu <sandeepa.s.prabhu@gmail.com>
Subject: Re: [BUGFIX PATCH 3/3] kprobes/arm: Fix the return address of multiple kretprobes
Date: Wed, 15 Feb 2017 01:01:07 +0900	[thread overview]
Message-ID: <20170215010107.8aff7cb6f526502daf094879@kernel.org> (raw)
In-Reply-To: <1487080027.2978.8.camel@linaro.org>

On Tue, 14 Feb 2017 13:47:07 +0000
"Jon Medhurst (Tixy)" <tixy@linaro.org> wrote:

> On Tue, 2017-02-14 at 10:32 +0000, Jon Medhurst (Tixy) wrote:
> > On Tue, 2017-02-14 at 00:05 +0900, Masami Hiramatsu wrote:
> > > This is arm port of commit 737480a0d525 ("kprobes/x86:
> > > Fix the return address of multiple kretprobes").
> > > 
> > > Fix the return address of subsequent kretprobes when multiple
> > > kretprobes are set on the same function.
> > > 
> > > For example:
> > > 
> > >   # cd /sys/kernel/debug/tracing
> > >   # echo "r:event1 sys_symlink" > kprobe_events
> > >   # echo "r:event2 sys_symlink" >> kprobe_events
> > >   # echo 1 > events/kprobes/enable
> > >   # ln -s /tmp/foo /tmp/bar
> > > 
> > >  (without this patch)
> > > 
> > >   # cat trace | grep -v ^#
> > >               ln-82    [000] dn.2    68.446525: event1: (kretprobe_trampoline+0x0/0x18 <- SyS_symlink)
> > >               ln-82    [000] dn.2    68.447831: event2: (ret_fast_syscall+0x0/0x1c <- SyS_symlink)
> > > 
> > >  (with this patch)
> > > 
> > >   # cat trace | grep -v ^#
> > >               ln-81    [000] dn.1    39.463469: event1: (ret_fast_syscall+0x0/0x1c <- SyS_symlink)
> > >               ln-81    [000] dn.1    39.464701: event2: (ret_fast_syscall+0x0/0x1c <- SyS_symlink)
> > > 
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > Cc: KUMANO Syuhei <kumano.prog@gmail.com>
> > > ---
> > 
> > I don't fully understand this function, but I've checked that the ARM
> > version now matches the x86 version (apart from the x86 specific
> > register fixup and some comments). So, FWIW
> > 
> > Acked-by: Jon Medhurst <tixy@linaro.org>
> > 
> > I ran the before and after test case in the commit log on ARM and
> > verified the result is correct. I also tried running the ARM kprobe
> > tests with these 3 fixes but the tests fail. However, they also fail
> > without any of these changes, so I'll investigate that further...
> 
> Bisecting the issue led me back to Linux 4.5 and commit 25362dc496ed
> ("ARM: 8501/1: mm: flip priority of CONFIG_DEBUG_RODATA")
> 
> This sets CONFIG_DEBUG_RODATA to be enabled by default. If I disable
> that on 4.10-rc4, with the three patches in this series, then the ARM
> kprobes tests pass OK.
> 
> I'll stick the DEBUG_RODATA issue on my todo list (it's been around for
> a year, so can probably wait a little longer).

Hmm, I'm running arm kernel on qemu, which maybe the reason why
the test case passed in my environment, since my kconfig also sets
CONFIG_DEBUG_RODATA=y.

BTW, would you see that any kprobe_events didn't work with
CONFIG_DEBUG_RODATA=y? (what the failure messages were?)

Thank you,

> 
> -- 
> Tixy
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

  reply	other threads:[~2017-02-14 16:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-13 15:02 [BUGFIX PATCH 0/3] kprobes/arm: Improve kprobes implementation on arm Masami Hiramatsu
2017-02-13 15:03 ` [BUGFIX PATCH 1/3] kprobes/arm: Allow to handle reentered kprobe on single-stepping Masami Hiramatsu
2017-02-14 10:01   ` Jon Medhurst (Tixy)
2017-02-14 15:32     ` Masami Hiramatsu
2017-02-13 15:04 ` [BUGFIX PATCH 2/3] kprobes/arm: Skip single-stepping in recursing path if possible Masami Hiramatsu
2017-02-14 10:07   ` Jon Medhurst (Tixy)
2017-02-14 15:31     ` Masami Hiramatsu
2017-02-13 15:05 ` [BUGFIX PATCH 3/3] kprobes/arm: Fix the return address of multiple kretprobes Masami Hiramatsu
2017-02-14 10:32   ` Jon Medhurst (Tixy)
2017-02-14 13:47     ` Jon Medhurst (Tixy)
2017-02-14 16:01       ` Masami Hiramatsu [this message]
2017-02-14 16:39         ` Jon Medhurst (Tixy)
2017-02-14 23:55           ` Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170215010107.8aff7cb6f526502daf094879@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=ananth@linux.vnet.ibm.com \
    --cc=catalin.marinas@arm.com \
    --cc=dave.long@linaro.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=sandeepa.s.prabhu@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=tixy@linaro.org \
    --cc=wangnan0@huawei.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.