From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Masami Hiramatsu <mhiramat@kernel.org>
Subject: [for-next][PATCH 12/12] kprobes: Add a test case for stacktrace from kretprobe handler
Date: Tue, 26 Oct 2021 12:23:27 -0400 [thread overview]
Message-ID: <20211026162349.585677978@goodmis.org> (raw)
In-Reply-To: 20211026162315.297389528@goodmis.org
From: Masami Hiramatsu <mhiramat@kernel.org>
Add a test case for stacktrace from kretprobe handler and
nested kretprobe handlers.
This test checks both of stack trace inside kretprobe handler
and stack trace from pt_regs. Those stack trace must include
actual function return address instead of kretprobe trampoline.
The nested kretprobe stacktrace test checks whether the unwinder
can correctly unwind the call frame on the stack which has been
modified by the kretprobe.
Since the stacktrace on kretprobe is correctly fixed only on x86,
this introduces a meta kconfig ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
which tells user that the stacktrace on kretprobe is correct or not.
The test results will be shown like below;
TAP version 14
1..1
# Subtest: kprobes_test
1..6
ok 1 - test_kprobe
ok 2 - test_kprobes
ok 3 - test_kretprobe
ok 4 - test_kretprobes
ok 5 - test_stacktrace_on_kretprobe
ok 6 - test_stacktrace_on_nested_kretprobe
# kprobes_test: pass:6 fail:0 skip:0 total:6
# Totals: pass:6 fail:0 skip:0 total:6
ok 1 - kprobes_test
Link: https://lkml.kernel.org/r/163516211244.604541.18350507860972214415.stgit@devnote2
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/Kconfig | 8 +++
arch/x86/Kconfig | 1 +
kernel/test_kprobes.c | 162 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 171 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index 8df1c7102643..8378f83b462c 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -191,6 +191,14 @@ config HAVE_OPTPROBES
config HAVE_KPROBES_ON_FTRACE
bool
+config ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
+ bool
+ help
+ Since kretprobes modifies return address on the stack, the
+ stacktrace may see the kretprobe trampoline address instead
+ of correct one. If the architecture stacktrace code and
+ unwinder can adjust such entries, select this configuration.
+
config HAVE_FUNCTION_ERROR_INJECTION
bool
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index e39e2bd6acc5..cc9ba6798848 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -61,6 +61,7 @@ config X86
select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
select ARCH_32BIT_OFF_T if X86_32
select ARCH_CLOCKSOURCE_INIT
+ select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
select ARCH_ENABLE_HUGEPAGE_MIGRATION if X86_64 && HUGETLB_PAGE && MIGRATION
select ARCH_ENABLE_MEMORY_HOTPLUG if X86_64 || (X86_32 && HIGHMEM)
select ARCH_ENABLE_MEMORY_HOTREMOVE if MEMORY_HOTPLUG
diff --git a/kernel/test_kprobes.c b/kernel/test_kprobes.c
index e78f18144145..a5edc2ebc947 100644
--- a/kernel/test_kprobes.c
+++ b/kernel/test_kprobes.c
@@ -17,6 +17,11 @@ static u32 (*target)(u32 value);
static u32 (*target2)(u32 value);
static struct kunit *current_test;
+static unsigned long (*internal_target)(void);
+static unsigned long (*stacktrace_target)(void);
+static unsigned long (*stacktrace_driver)(void);
+static unsigned long target_return_address[2];
+
static noinline u32 kprobe_target(u32 value)
{
return (value / div_factor);
@@ -58,6 +63,33 @@ static noinline u32 kprobe_target2(u32 value)
return (value / div_factor) + 1;
}
+static noinline unsigned long kprobe_stacktrace_internal_target(void)
+{
+ if (!target_return_address[0])
+ target_return_address[0] = (unsigned long)__builtin_return_address(0);
+ return target_return_address[0];
+}
+
+static noinline unsigned long kprobe_stacktrace_target(void)
+{
+ if (!target_return_address[1])
+ target_return_address[1] = (unsigned long)__builtin_return_address(0);
+
+ if (internal_target)
+ internal_target();
+
+ return target_return_address[1];
+}
+
+static noinline unsigned long kprobe_stacktrace_driver(void)
+{
+ if (stacktrace_target)
+ stacktrace_target();
+
+ /* This is for preventing inlining the function */
+ return (unsigned long)__builtin_return_address(0);
+}
+
static int kp_pre_handler2(struct kprobe *p, struct pt_regs *regs)
{
preh_val = (rand1 / div_factor) + 1;
@@ -175,12 +207,138 @@ static void test_kretprobes(struct kunit *test)
KUNIT_EXPECT_EQ(test, krph_val, rand1);
unregister_kretprobes(rps, 2);
}
+
+#ifdef CONFIG_ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
+#define STACK_BUF_SIZE 16
+static unsigned long stack_buf[STACK_BUF_SIZE];
+
+static int stacktrace_return_handler(struct kretprobe_instance *ri, struct pt_regs *regs)
+{
+ unsigned long retval = regs_return_value(regs);
+ int i, ret;
+
+ KUNIT_EXPECT_FALSE(current_test, preemptible());
+ KUNIT_EXPECT_EQ(current_test, retval, target_return_address[1]);
+
+ /*
+ * Test stacktrace inside the kretprobe handler, this will involves
+ * kretprobe trampoline, but must include correct return address
+ * of the target function.
+ */
+ ret = stack_trace_save(stack_buf, STACK_BUF_SIZE, 0);
+ KUNIT_EXPECT_NE(current_test, ret, 0);
+
+ for (i = 0; i < ret; i++) {
+ if (stack_buf[i] == target_return_address[1])
+ break;
+ }
+ KUNIT_EXPECT_NE(current_test, i, ret);
+
+#if !IS_MODULE(CONFIG_KPROBES_SANITY_TEST)
+ /*
+ * Test stacktrace from pt_regs at the return address. Thus the stack
+ * trace must start from the target return address.
+ */
+ ret = stack_trace_save_regs(regs, stack_buf, STACK_BUF_SIZE, 0);
+ KUNIT_EXPECT_NE(current_test, ret, 0);
+ KUNIT_EXPECT_EQ(current_test, stack_buf[0], target_return_address[1]);
+#endif
+
+ return 0;
+}
+
+static struct kretprobe rp3 = {
+ .handler = stacktrace_return_handler,
+ .kp.symbol_name = "kprobe_stacktrace_target"
+};
+
+static void test_stacktrace_on_kretprobe(struct kunit *test)
+{
+ unsigned long myretaddr = (unsigned long)__builtin_return_address(0);
+
+ current_test = test;
+ rp3.kp.addr = NULL;
+ rp3.kp.flags = 0;
+
+ /*
+ * Run the stacktrace_driver() to record correct return address in
+ * stacktrace_target() and ensure stacktrace_driver() call is not
+ * inlined by checking the return address of stacktrace_driver()
+ * and the return address of this function is different.
+ */
+ KUNIT_ASSERT_NE(test, myretaddr, stacktrace_driver());
+
+ KUNIT_ASSERT_EQ(test, 0, register_kretprobe(&rp3));
+ KUNIT_ASSERT_NE(test, myretaddr, stacktrace_driver());
+ unregister_kretprobe(&rp3);
+}
+
+static int stacktrace_internal_return_handler(struct kretprobe_instance *ri, struct pt_regs *regs)
+{
+ unsigned long retval = regs_return_value(regs);
+ int i, ret;
+
+ KUNIT_EXPECT_FALSE(current_test, preemptible());
+ KUNIT_EXPECT_EQ(current_test, retval, target_return_address[0]);
+
+ /*
+ * Test stacktrace inside the kretprobe handler for nested case.
+ * The unwinder will find the kretprobe_trampoline address on the
+ * return address, and kretprobe must solve that.
+ */
+ ret = stack_trace_save(stack_buf, STACK_BUF_SIZE, 0);
+ KUNIT_EXPECT_NE(current_test, ret, 0);
+
+ for (i = 0; i < ret - 1; i++) {
+ if (stack_buf[i] == target_return_address[0]) {
+ KUNIT_EXPECT_EQ(current_test, stack_buf[i + 1], target_return_address[1]);
+ break;
+ }
+ }
+ KUNIT_EXPECT_NE(current_test, i, ret);
+
+#if !IS_MODULE(CONFIG_KPROBES_SANITY_TEST)
+ /* Ditto for the regs version. */
+ ret = stack_trace_save_regs(regs, stack_buf, STACK_BUF_SIZE, 0);
+ KUNIT_EXPECT_NE(current_test, ret, 0);
+ KUNIT_EXPECT_EQ(current_test, stack_buf[0], target_return_address[0]);
+ KUNIT_EXPECT_EQ(current_test, stack_buf[1], target_return_address[1]);
+#endif
+
+ return 0;
+}
+
+static struct kretprobe rp4 = {
+ .handler = stacktrace_internal_return_handler,
+ .kp.symbol_name = "kprobe_stacktrace_internal_target"
+};
+
+static void test_stacktrace_on_nested_kretprobe(struct kunit *test)
+{
+ unsigned long myretaddr = (unsigned long)__builtin_return_address(0);
+ struct kretprobe *rps[2] = {&rp3, &rp4};
+
+ current_test = test;
+ rp3.kp.addr = NULL;
+ rp3.kp.flags = 0;
+
+ //KUNIT_ASSERT_NE(test, myretaddr, stacktrace_driver());
+
+ KUNIT_ASSERT_EQ(test, 0, register_kretprobes(rps, 2));
+ KUNIT_ASSERT_NE(test, myretaddr, stacktrace_driver());
+ unregister_kretprobes(rps, 2);
+}
+#endif /* CONFIG_ARCH_CORRECT_STACKTRACE_ON_KRETPROBE */
+
#endif /* CONFIG_KRETPROBES */
static int kprobes_test_init(struct kunit *test)
{
target = kprobe_target;
target2 = kprobe_target2;
+ stacktrace_target = kprobe_stacktrace_target;
+ internal_target = kprobe_stacktrace_internal_target;
+ stacktrace_driver = kprobe_stacktrace_driver;
do {
rand1 = prandom_u32();
@@ -194,6 +352,10 @@ static struct kunit_case kprobes_testcases[] = {
#ifdef CONFIG_KRETPROBES
KUNIT_CASE(test_kretprobe),
KUNIT_CASE(test_kretprobes),
+#ifdef CONFIG_ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
+ KUNIT_CASE(test_stacktrace_on_kretprobe),
+ KUNIT_CASE(test_stacktrace_on_nested_kretprobe),
+#endif
#endif
{}
};
--
2.33.0
prev parent reply other threads:[~2021-10-26 16:24 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-26 16:23 [for-next][PATCH 00/12] tracing: More updates for 5.16 Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 01/12] ftrace: Make ftrace_profile_pages_init static Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 02/12] tools/latency-collector: Use correct size when writing queue_full_warning Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 03/12] perf/core: allow ftrace for functions in kernel/event/core.c Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 04/12] trace/osnoise: Fix an ifdef comment Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 05/12] tracing/doc: Fix typos on the timerlat tracer documentation Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 06/12] trace/osnoise: Add migrate-disabled field to the osnoise header Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 07/12] trace/timerlat: Add migrate-disabled field to the timerlat header Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 08/12] tracing: Fix missing trace_boot_init_histograms kstrdup NULL checks Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 09/12] tracing/hwlat: Make some internal symbols static Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 10/12] ftrace/sh: Add arch_ftrace_ops_list_func stub to have compressed image still link Steven Rostedt
2021-10-27 8:58 ` Sergey Shtylyov
2021-10-27 12:01 ` Steven Rostedt
2021-10-26 16:23 ` [for-next][PATCH 11/12] lib/bootconfig: Make xbc_alloc_mem() and xbc_free_mem() as __init function Steven Rostedt
2021-10-26 16:23 ` Steven Rostedt [this message]
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=20211026162349.585677978@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox