linux-kernel - Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc

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:   Sat, 24 Jun 2017 20:06:10 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        linux-kernel@...r.kernel.org,
        Masami Hiramatsu <mhiramat@...nel.org>,
        linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for
 powerpc

On Sat, 24 Jun 2017 02:30:21 +0900
Masami Hiramatsu <mhiramat@...nel.org> wrote:

> On Thu, 22 Jun 2017 22:33:25 +0530
> "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com> wrote:
> 
> > On 2017/06/22 06:07PM, Masami Hiramatsu wrote:
> > > On Thu, 22 Jun 2017 00:20:28 +0530
> > > "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com> wrote:
> > > 
> > > > KPROBES_ON_FTRACE is only available on powerpc64le. Update comment to
> > > > clarify this.
> > > > 
> > > > Also, we should use an offset of 8 to ensure that the probe does not
> > > > fall on ftrace location. The current offset of 4 will fall before the
> > > > function local entry point and won't fire, while an offset of 12 or 16
> > > > will fall on ftrace location. Offset 8 is currently guaranteed to not be
> > > > the ftrace location.
> > > 
> > > OK, these part seems good to me.
> > > 
> > > > 
> > > > Finally, do not filter out symbols with a dot. Powerpc Elfv1 uses dot
> > > > prefix for all functions and this prevents us from testing some of those
> > > > symbols. Furthermore, with the patch to derive event names properly in
> > > > the presence of ':' and '.', such names are accepted by kprobe_events
> > > > and constitutes a good test for those symbols.
> > > 
> > > Hmm, the reason why I added such filter was to avoid symbols including
> > > gcc-generated suffixes like as .constprop or .isra etc.
> > 
> > I see.
> > 
> > I do wonder -- is there a problem if we try probing those symbols? On my 
> > local x86 vm, I don't see an issue probing it especially with the 
> > previous patch to enable probing with symbols having a '.' or ':'.
> > 
> > Furthermore, since this is for testing kprobe_events, I feel it is good 
> > to try probing those symbols too to catch any weird errors we may hit.
> 
> Yes, and that is not what this testcase is aiming to. That testcase should
> be a separated one, with correct error handling.

Hi Naveen,

Here is the testcase which I meant above. This may help if there is any
regression related to this specific issue.

Thank you,

-----

selftests: ftrace: Add a testcase for kprobe event naming

From: Masami Hiramatsu <mhiramat@...nel.org>

Add a testcase for kprobe event naming. This testcase
checks whether the kprobe events can automatically ganerate
its event name on normal function and dot-suffixed function.
Also it checks whether the kprobe events can correctly
define new event with given event name and group name.

Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org>
---
 .../ftrace/test.d/kprobe/kprobe_eventname.tc       |   28 ++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc
new file mode 100644
index 0000000..d259031
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc
@@ -0,0 +1,28 @@
+#!/bin/sh
+# description: Kprobe event auto/manual naming
+
+disable_events
+echo > kprobe_events
+
+:;: "Add an event on function without name" ;:
+
+FUNC=`grep -m 10 " [tT] [^.]*$" /proc/kallsyms | tail -n 1 | cut -f 3 -d " "`
+echo p $FUNC > kprobe_events
+test -d events/kprobes/p_${FUNC}_0 || exit_failure
+
+:;: "Add an event on function with new name" ;:
+
+echo p:event1 $FUNC > kprobe_events
+test -d events/kprobes/event1 || exit_failure
+
+:;: "Add an event on function with new name and group" ;:
+
+echo p:kprobes2/event2 $FUNC > kprobe_events
+test -d events/kprobes2/event2 || exit_failure
+
+:;: "Add an event on dot function without name" ;:
+
+FUNC=`grep -m 10 " [tT] .*\..*$" /proc/kallsyms | tail -n 1 | cut -f 3 -d " "`
+echo p $FUNC > kprobe_events
+EVENT=`grep $FUNC kprobe_events | cut -f 1 -d " " | cut -f 2 -d:` || exit_failure
+test -d events/$EVENT || exit_failure
-- 
Masami Hiramatsu <mhiramat@...nel.org>