[PATCH v5 0/3] tracing: kprobes: Prohibit probing on notrace functions

[PATCH v5 0/3] tracing: kprobes: Prohibit probing on notrace functions

[Masami Hiramatsu]

Hi,

This is the 5th version of the series to prohibit kprobe
on notrace functions.

This fixes a build error when CONFIG_FUNCTION_TRACER=y but
DYNAMIC_FTRACE=n. Finally I decided to recover the approach
in the 1st version, made it depends on CONFIG_KPROBES_ON_FTRACE=y.
It is the simplest and enough to prohibiting kernel crash.

So, in summary, if CONFIG_KPROBES_ON_FTRACE=y (which depends
on CONFIG_DYNAMIC_FTRACE=y, so ftrace_location_range must be
there) && CONFIG_KPROBE_EVENTS_ON_NOTRACE=n (default),
kprobe events can not be defined on notrace function.
Otherwides (means CONFIG_KPROBES_ON_FTRACE=n or
CONFIG_KPROBE_EVENTS_ON_NOTRACE=y), we can put kprobe events
on notrace functions.

Francis, I dropped your tested-by because I got many kbuild
errors and fixed it. If you can, could you test it again?

Thank you,

---

Francis Deslauriers (1):
      selftest/ftrace: Move kprobe selftest function to separate compile unit

Masami Hiramatsu (2):
      tracing: kprobes: Prohibit probing on notrace function
      selftests/ftrace: Fix kprobe string testcase to not probe notrace function


 kernel/trace/Kconfig                               |   20 +++++++
 kernel/trace/Makefile                              |    5 ++
 kernel/trace/trace_kprobe.c                        |   59 +++++++++++++-------
 kernel/trace/trace_kprobe_selftest.c               |   10 +++
 kernel/trace/trace_kprobe_selftest.h               |    7 ++
 .../ftrace/test.d/kprobe/kprobe_args_string.tc     |   30 ++++------
 .../selftests/ftrace/test.d/kprobe/probepoint.tc   |    2 -
 7 files changed, 94 insertions(+), 39 deletions(-)
 create mode 100644 kernel/trace/trace_kprobe_selftest.c
 create mode 100644 kernel/trace/trace_kprobe_selftest.h

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[PATCH v5 1/3] tracing: kprobes: Prohibit probing on notrace function

[Masami Hiramatsu]

Prohibit kprobe-events probing on notrace function.
Since probing on the notrace function can cause recursive
event call. In most case those are just skipped, but
in some case it falls into infinit recursive call.

This protection can be disabled by the kconfig
CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, but it is highly
recommended to keep it "n" for normal kernel.
Note that this is only available if "kprobes on ftrace"
has been implemented on target arch and
CONFIG_KPROBES_ON_FTRACE=y.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
  Changes in v2
   - Add CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig for knocking down
     the protection.
  Changes in v3
   - Fix to check raw-address (no symbol) probe point correctly.
  Changes in v4
   - No notrace check if CONFIG_FUNCTION_TRACER=n. In that case
     notrace is ignored.
  Changes in v5
   - Change dependency on CONFIG_KPROBES_ON_FTRACE as same as
     original v1 patch does.
---
 kernel/trace/Kconfig        |   20 ++++++++++++++++++
 kernel/trace/trace_kprobe.c |   47 +++++++++++++++++++++++++++++++++++--------
 2 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index dcc0166d1997..88b39f91b1e3 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -456,6 +456,26 @@ config KPROBE_EVENTS
 	  This option is also required by perf-probe subcommand of perf tools.
 	  If you want to use perf tools, this option is strongly recommended.
 
+config KPROBE_EVENTS_ON_NOTRACE
+	bool "Do NOT protect notrace function from kprobe events"
+	depends on KPROBE_EVENTS
+	depends on KPROBES_ON_FTRACE
+	default n
+	help
+	  This is only for the developers who want to debug ftrace itself
+	  using kprobe events.
+
+	  If kprobes can use ftrace instead of breakpoint, ftrace related
+	  functions are protected from kprobe-events to prevent an infinit
+	  recursion or any unexpected execution path which leads to a kernel
+	  crash.
+
+	  This option disables such protection and allows you to put kprobe
+	  events on ftrace functions for debugging ftrace by itself.
+	  Note that this might let you shoot yourself in the foot.
+
+	  If unsure, say N.
+
 config UPROBE_EVENTS
 	bool "Enable uprobes-based dynamic events"
 	depends on ARCH_SUPPORTS_UPROBES
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index b37b92e7dbd4..10fdf1605e01 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -87,6 +87,21 @@ static nokprobe_inline unsigned long trace_kprobe_nhit(struct trace_kprobe *tk)
 	return nhit;
 }
 
+static nokprobe_inline
+unsigned long trace_kprobe_address(struct trace_kprobe *tk)
+{
+	unsigned long addr;
+
+	if (tk->symbol) {
+		addr = (unsigned long)
+			kallsyms_lookup_name(trace_kprobe_symbol(tk));
+		addr += tk->rp.kp.offset;
+	} else {
+		addr = (unsigned long)tk->rp.kp.addr;
+	}
+	return addr;
+}
+
 bool trace_kprobe_on_func_entry(struct trace_event_call *call)
 {
 	struct trace_kprobe *tk = (struct trace_kprobe *)call->data;
@@ -99,16 +114,8 @@ bool trace_kprobe_on_func_entry(struct trace_event_call *call)
 bool trace_kprobe_error_injectable(struct trace_event_call *call)
 {
 	struct trace_kprobe *tk = (struct trace_kprobe *)call->data;
-	unsigned long addr;
 
-	if (tk->symbol) {
-		addr = (unsigned long)
-			kallsyms_lookup_name(trace_kprobe_symbol(tk));
-		addr += tk->rp.kp.offset;
-	} else {
-		addr = (unsigned long)tk->rp.kp.addr;
-	}
-	return within_error_injection_list(addr);
+	return within_error_injection_list(trace_kprobe_address(tk));
 }
 
 static int register_kprobe_event(struct trace_kprobe *tk);
@@ -487,6 +494,22 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct trace_event_file *file)
 	return ret;
 }
 
+#if defined(CONFIG_KPROBES_ON_FTRACE) && \
+	!defined(CONFIG_KPROBE_EVENTS_ON_NOTRACE)
+static bool within_notrace_func(struct trace_kprobe *tk)
+{
+	unsigned long offset, size, addr;
+
+	addr = trace_kprobe_address(tk);
+	if (!kallsyms_lookup_size_offset(addr, &size, &offset))
+		return true;	/* Out of range. */
+
+	return !ftrace_location_range(addr - offset, addr - offset + size);
+}
+#else
+#define within_notrace_func(tk)	(false)
+#endif
+
 /* Internal register function - just handle k*probes and flags */
 static int __register_trace_kprobe(struct trace_kprobe *tk)
 {
@@ -495,6 +518,12 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
 	if (trace_probe_is_registered(&tk->tp))
 		return -EINVAL;
 
+	if (within_notrace_func(tk)) {
+		pr_warn("Could not probe notrace function %s\n",
+			trace_kprobe_symbol(tk));
+		return -EINVAL;
+	}
+
 	for (i = 0; i < tk->tp.nr_args; i++)
 		traceprobe_update_arg(&tk->tp.args[i]);
 

[PATCH v5 2/3] selftest/ftrace: Move kprobe selftest function to separate compile unit

[Masami Hiramatsu]

From: Francis Deslauriers <francis.deslauriers@efficios.com>

Move selftest function to its own compile unit so it can be compiled
with the ftrace cflags (CC_FLAGS_FTRACE) allowing it to be probed
during the ftrace startup tests.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/Makefile                |    5 +++++
 kernel/trace/trace_kprobe.c          |   12 +-----------
 kernel/trace/trace_kprobe_selftest.c |   10 ++++++++++
 kernel/trace/trace_kprobe_selftest.h |    7 +++++++
 4 files changed, 23 insertions(+), 11 deletions(-)
 create mode 100644 kernel/trace/trace_kprobe_selftest.c
 create mode 100644 kernel/trace/trace_kprobe_selftest.h

diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index e2538c7638d4..e38771eccb2f 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -13,6 +13,11 @@ obj-y += trace_selftest_dynamic.o
 endif
 endif
 
+ifdef CONFIG_FTRACE_STARTUP_TEST
+CFLAGS_trace_kprobe_selftest.o = $(CC_FLAGS_FTRACE)
+obj-$(CONFIG_KPROBE_EVENTS) += trace_kprobe_selftest.o
+endif
+
 # If unlikely tracing is enabled, do not trace these files
 ifdef CONFIG_TRACING_BRANCHES
 KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 10fdf1605e01..d314b843beca 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -23,6 +23,7 @@
 #include <linux/rculist.h>
 #include <linux/error-injection.h>
 
+#include "trace_kprobe_selftest.h"
 #include "trace_probe.h"
 
 #define KPROBE_EVENT_SYSTEM "kprobes"
@@ -1565,17 +1566,6 @@ fs_initcall(init_kprobe_trace);
 
 
 #ifdef CONFIG_FTRACE_STARTUP_TEST
-/*
- * The "__used" keeps gcc from removing the function symbol
- * from the kallsyms table. 'noinline' makes sure that there
- * isn't an inlined version used by the test method below
- */
-static __used __init noinline int
-kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int a6)
-{
-	return a1 + a2 + a3 + a4 + a5 + a6;
-}
-
 static __init struct trace_event_file *
 find_trace_probe_file(struct trace_kprobe *tk, struct trace_array *tr)
 {
diff --git a/kernel/trace/trace_kprobe_selftest.c b/kernel/trace/trace_kprobe_selftest.c
new file mode 100644
index 000000000000..16548ee4c8c6
--- /dev/null
+++ b/kernel/trace/trace_kprobe_selftest.c
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Function used during the kprobe self test. This function is in a separate
+ * compile unit so it can be compile with CC_FLAGS_FTRACE to ensure that it
+ * can be probed by the selftests.
+ */
+int kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int a6)
+{
+	return a1 + a2 + a3 + a4 + a5 + a6;
+}
diff --git a/kernel/trace/trace_kprobe_selftest.h b/kernel/trace/trace_kprobe_selftest.h
new file mode 100644
index 000000000000..4e10ec41c013
--- /dev/null
+++ b/kernel/trace/trace_kprobe_selftest.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Function used during the kprobe self test. This function is in a separate
+ * compile unit so it can be compile with CC_FLAGS_FTRACE to ensure that it
+ * can be probed by the selftests.
+ */
+int kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int a6);

[PATCH v5 3/3] selftests/ftrace: Fix kprobe string testcase to not probe notrace function

[Masami Hiramatsu]

Fix kprobe string argument testcase to not probe notrace
function. Instead, it probes tracefs function which must
be available with ftrace.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Changes in v3:
  - Fix probepoint testcase too.
---
 .../ftrace/test.d/kprobe/kprobe_args_string.tc     |   30 ++++++++------------
 .../selftests/ftrace/test.d/kprobe/probepoint.tc   |    2 +
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
index a0002563e9ee..1ad70cdaf442 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
@@ -9,28 +9,22 @@ echo > kprobe_events
 
 case `uname -m` in
 x86_64)
-  ARG2=%si
-  OFFS=8
+  ARG1=%di
 ;;
 i[3456]86)
-  ARG2=%cx
-  OFFS=4
+  ARG1=%ax
 ;;
 aarch64)
-  ARG2=%x1
-  OFFS=8
+  ARG1=%x0
 ;;
 arm*)
-  ARG2=%r1
-  OFFS=4
+  ARG1=%r0
 ;;
 ppc64*)
-  ARG2=%r4
-  OFFS=8
+  ARG1=%r3
 ;;
 ppc*)
-  ARG2=%r4
-  OFFS=4
+  ARG1=%r3
 ;;
 *)
   echo "Please implement other architecture here"
@@ -38,17 +32,17 @@ ppc*)
 esac
 
 : "Test get argument (1)"
-echo "p:testprobe create_trace_kprobe arg1=+0(+0(${ARG2})):string" > kprobe_events
+echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string" > kprobe_events
 echo 1 > events/kprobes/testprobe/enable
-! echo test >> kprobe_events
-tail -n 1 trace | grep -qe "testprobe.* arg1=\"test\""
+echo "p:test _do_fork" >> kprobe_events
+grep -qe "testprobe.* arg1=\"test\"" trace
 
 echo 0 > events/kprobes/testprobe/enable
 : "Test get argument (2)"
-echo "p:testprobe create_trace_kprobe arg1=+0(+0(${ARG2})):string arg2=+0(+${OFFS}(${ARG2})):string" > kprobe_events
+echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string arg2=+0(${ARG1}):string" > kprobe_events
 echo 1 > events/kprobes/testprobe/enable
-! echo test1 test2 >> kprobe_events
-tail -n 1 trace | grep -qe "testprobe.* arg1=\"test1\" arg2=\"test2\""
+echo "p:test _do_fork" >> kprobe_events
+grep -qe "testprobe.* arg1=\"test\" arg2=\"test\"" trace
 
 echo 0 > events/enable
 echo > kprobe_events
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc b/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc
index 4fda01a08da4..519d2763f5d2 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/probepoint.tc
@@ -4,7 +4,7 @@
 
 [ -f kprobe_events ] || exit_unsupported # this is configurable
 
-TARGET_FUNC=create_trace_kprobe
+TARGET_FUNC=tracefs_create_dir
 
 dec_addr() { # hexaddr
   printf "%d" "0x"`echo $1 | tail -c 8`

Re: [PATCH v5 0/3] tracing: kprobes: Prohibit probing on notrace functions

[Francis Deslauriers]

Hi Masami,
I just tested the patch-set and it still prevents the crash I was witnessing.
You can add my tested-by.
Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com>

Thank you for pushing this forward!
Francis
Le lun. 30 juill. 2018, à 06 h 20, Masami Hiramatsu
<mhiramat@kernel.org> a écrit :
>
> Hi,
>
> This is the 5th version of the series to prohibit kprobe
> on notrace functions.
>
> This fixes a build error when CONFIG_FUNCTION_TRACER=y but
> DYNAMIC_FTRACE=n. Finally I decided to recover the approach
> in the 1st version, made it depends on CONFIG_KPROBES_ON_FTRACE=y.
> It is the simplest and enough to prohibiting kernel crash.
>
> So, in summary, if CONFIG_KPROBES_ON_FTRACE=y (which depends
> on CONFIG_DYNAMIC_FTRACE=y, so ftrace_location_range must be
> there) && CONFIG_KPROBE_EVENTS_ON_NOTRACE=n (default),
> kprobe events can not be defined on notrace function.
> Otherwides (means CONFIG_KPROBES_ON_FTRACE=n or
> CONFIG_KPROBE_EVENTS_ON_NOTRACE=y), we can put kprobe events
> on notrace functions.
>
> Francis, I dropped your tested-by because I got many kbuild
> errors and fixed it. If you can, could you test it again?
>
> Thank you,
>
> ---
>
> Francis Deslauriers (1):
>       selftest/ftrace: Move kprobe selftest function to separate compile unit
>
> Masami Hiramatsu (2):
>       tracing: kprobes: Prohibit probing on notrace function
>       selftests/ftrace: Fix kprobe string testcase to not probe notrace function
>
>
>  kernel/trace/Kconfig                               |   20 +++++++
>  kernel/trace/Makefile                              |    5 ++
>  kernel/trace/trace_kprobe.c                        |   59 +++++++++++++-------
>  kernel/trace/trace_kprobe_selftest.c               |   10 +++
>  kernel/trace/trace_kprobe_selftest.h               |    7 ++
>  .../ftrace/test.d/kprobe/kprobe_args_string.tc     |   30 ++++------
>  .../selftests/ftrace/test.d/kprobe/probepoint.tc   |    2 -
>  7 files changed, 94 insertions(+), 39 deletions(-)
>  create mode 100644 kernel/trace/trace_kprobe_selftest.c
>  create mode 100644 kernel/trace/trace_kprobe_selftest.h
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>



--
Francis Deslauriers
Software developer
EfficiOS inc.

Re: [PATCH v5 1/3] tracing: kprobes: Prohibit probing on notrace function

[Steven Rostedt]

On Mon, 30 Jul 2018 19:20:14 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Prohibit kprobe-events probing on notrace function.
> Since probing on the notrace function can cause recursive
> event call. In most case those are just skipped, but
> in some case it falls into infinit recursive call.
> 
> This protection can be disabled by the kconfig
> CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, but it is highly
> recommended to keep it "n" for normal kernel.
> Note that this is only available if "kprobes on ftrace"
> has been implemented on target arch and
> CONFIG_KPROBES_ON_FTRACE=y.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>

Hi Masami,

Note, I made the following changes for grammar. For the Change log I
have this:

    tracing: kprobes: Prohibit probing on notrace function
    
    Prohibit kprobe-events probing on notrace functions.  Since probing on a
    notrace function can cause a recursive event call. In most cases those are just
    skipped, but in some cases it falls into an infinite recursive call.
    
    This protection can be disabled by the kconfig
    CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, but it is highly recommended to keep it
    "n" for normal kernel builds.  Note that this is only available if "kprobes on
    ftrace" has been implemented on the target arch and CONFIG_KPROBES_ON_FTRACE=y.
    
    Link: http://lkml.kernel.org/r/153294601436.32740.10557881188933661239.stgit@devbox
    
    Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
    Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com>
    [ Slight grammar and spelling fixes ]
    Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>


And this change to the patch:

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 4d4eb15cc7fd..23fc7c9abedb 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -457,7 +457,7 @@ config KPROBE_EVENTS
 	  If you want to use perf tools, this option is strongly recommended.
 
 config KPROBE_EVENTS_ON_NOTRACE
-	bool "Do NOT protect notrace function from kprobe events"
+	bool "Do NOT protect notrace functions from kprobe events"
 	depends on KPROBE_EVENTS
 	depends on KPROBES_ON_FTRACE
 	default n
@@ -465,13 +465,13 @@ config KPROBE_EVENTS_ON_NOTRACE
 	  This is only for the developers who want to debug ftrace itself
 	  using kprobe events.
 
-	  If kprobes can use ftrace instead of breakpoint, ftrace related
-	  functions are protected from kprobe-events to prevent an infinit
-	  recursion or any unexpected execution path which leads to a kernel
-	  crash.
+	  If kprobes is using ftrace to hook to a function instead of a
+	  breakpoint, ftrace related functions are protected from
+	  kprobe-events to prevent an infinite recursion or any unexpected
+	  execution path which would lead to a kernel crash.
 
 	  This option disables such protection and allows you to put kprobe
-	  events on ftrace functions for debugging ftrace by itself.
+	  events on ftrace functions for debugging ftrace itself.
 	  Note that this might let you shoot yourself in the foot.
 
 	  If unsure, say N.


Are you OK with this?

-- Steve

Re: [PATCH v5 0/3] tracing: kprobes: Prohibit probing on notrace functions

[Masami Hiramatsu]

On Mon, 30 Jul 2018 12:06:17 -0400
Francis Deslauriers <francis.deslauriers@efficios.com> wrote:

> Hi Masami,
> I just tested the patch-set and it still prevents the crash I was witnessing.
> You can add my tested-by.
> Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com>

Thanks Francis!


> 
> Thank you for pushing this forward!
> Francis
> Le lun. 30 juill. 2018, à 06 h 20, Masami Hiramatsu
> <mhiramat@kernel.org> a écrit :
> >
> > Hi,
> >
> > This is the 5th version of the series to prohibit kprobe
> > on notrace functions.
> >
> > This fixes a build error when CONFIG_FUNCTION_TRACER=y but
> > DYNAMIC_FTRACE=n. Finally I decided to recover the approach
> > in the 1st version, made it depends on CONFIG_KPROBES_ON_FTRACE=y.
> > It is the simplest and enough to prohibiting kernel crash.
> >
> > So, in summary, if CONFIG_KPROBES_ON_FTRACE=y (which depends
> > on CONFIG_DYNAMIC_FTRACE=y, so ftrace_location_range must be
> > there) && CONFIG_KPROBE_EVENTS_ON_NOTRACE=n (default),
> > kprobe events can not be defined on notrace function.
> > Otherwides (means CONFIG_KPROBES_ON_FTRACE=n or
> > CONFIG_KPROBE_EVENTS_ON_NOTRACE=y), we can put kprobe events
> > on notrace functions.
> >
> > Francis, I dropped your tested-by because I got many kbuild
> > errors and fixed it. If you can, could you test it again?
> >
> > Thank you,
> >
> > ---
> >
> > Francis Deslauriers (1):
> >       selftest/ftrace: Move kprobe selftest function to separate compile unit
> >
> > Masami Hiramatsu (2):
> >       tracing: kprobes: Prohibit probing on notrace function
> >       selftests/ftrace: Fix kprobe string testcase to not probe notrace function
> >
> >
> >  kernel/trace/Kconfig                               |   20 +++++++
> >  kernel/trace/Makefile                              |    5 ++
> >  kernel/trace/trace_kprobe.c                        |   59 +++++++++++++-------
> >  kernel/trace/trace_kprobe_selftest.c               |   10 +++
> >  kernel/trace/trace_kprobe_selftest.h               |    7 ++
> >  .../ftrace/test.d/kprobe/kprobe_args_string.tc     |   30 ++++------
> >  .../selftests/ftrace/test.d/kprobe/probepoint.tc   |    2 -
> >  7 files changed, 94 insertions(+), 39 deletions(-)
> >  create mode 100644 kernel/trace/trace_kprobe_selftest.c
> >  create mode 100644 kernel/trace/trace_kprobe_selftest.h
> >
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> 
> 
> 
> --
> Francis Deslauriers
> Software developer
> EfficiOS inc.


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v5 1/3] tracing: kprobes: Prohibit probing on notrace function

[Masami Hiramatsu]

On Mon, 30 Jul 2018 18:40:10 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 30 Jul 2018 19:20:14 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Prohibit kprobe-events probing on notrace function.
> > Since probing on the notrace function can cause recursive
> > event call. In most case those are just skipped, but
> > in some case it falls into infinit recursive call.
> > 
> > This protection can be disabled by the kconfig
> > CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, but it is highly
> > recommended to keep it "n" for normal kernel.
> > Note that this is only available if "kprobes on ftrace"
> > has been implemented on target arch and
> > CONFIG_KPROBES_ON_FTRACE=y.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> >
> 
> Hi Masami,
> 
> Note, I made the following changes for grammar. For the Change log I
> have this:
> 
>     tracing: kprobes: Prohibit probing on notrace function
>     
>     Prohibit kprobe-events probing on notrace functions.  Since probing on a
>     notrace function can cause a recursive event call. In most cases those are just
>     skipped, but in some cases it falls into an infinite recursive call.
>     
>     This protection can be disabled by the kconfig
>     CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, but it is highly recommended to keep it
>     "n" for normal kernel builds.  Note that this is only available if "kprobes on
>     ftrace" has been implemented on the target arch and CONFIG_KPROBES_ON_FTRACE=y.
>     
>     Link: http://lkml.kernel.org/r/153294601436.32740.10557881188933661239.stgit@devbox
>     
>     Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>     Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com>
>     [ Slight grammar and spelling fixes ]
>     Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Looks good to me.
Thanks for fixing it! :)

> 
> 
> And this change to the patch:
> 
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 4d4eb15cc7fd..23fc7c9abedb 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -457,7 +457,7 @@ config KPROBE_EVENTS
>  	  If you want to use perf tools, this option is strongly recommended.
>  
>  config KPROBE_EVENTS_ON_NOTRACE
> -	bool "Do NOT protect notrace function from kprobe events"
> +	bool "Do NOT protect notrace functions from kprobe events"
>  	depends on KPROBE_EVENTS
>  	depends on KPROBES_ON_FTRACE
>  	default n
> @@ -465,13 +465,13 @@ config KPROBE_EVENTS_ON_NOTRACE
>  	  This is only for the developers who want to debug ftrace itself
>  	  using kprobe events.
>  
> -	  If kprobes can use ftrace instead of breakpoint, ftrace related
> -	  functions are protected from kprobe-events to prevent an infinit
> -	  recursion or any unexpected execution path which leads to a kernel
> -	  crash.
> +	  If kprobes is using ftrace to hook to a function instead of a
> +	  breakpoint, ftrace related functions are protected from
> +	  kprobe-events to prevent an infinite recursion or any unexpected
> +	  execution path which would lead to a kernel crash.
>  
>  	  This option disables such protection and allows you to put kprobe
> -	  events on ftrace functions for debugging ftrace by itself.
> +	  events on ftrace functions for debugging ftrace itself.
>  	  Note that this might let you shoot yourself in the foot.
>  
>  	  If unsure, say N.
> 
> 
> Are you OK with this?
> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>