Re: [PATCH] test_kprobes: clear kprobes between test runs

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Thu,  7 May 2026 15:44:33 +0200
Martin Kaiser <martin@kaiser.cx> wrote:

> Running the kprobes sanity tests twice makes all tests fail and
> eventually crashes the kernel.
> 
> [root@martin-riscv-1 ~]# echo 1 > /sys/kernel/debug/kunit/kprobes_test/run
> ...
>    # Totals: pass:5 fail:0 skip:0 total:5
>    ok 1 kprobes_test
> [root@martin-riscv-1 ~]# echo 1 > /sys/kernel/debug/kunit/kprobes_test/run
> ...
>   # test_kprobe: EXPECTATION FAILED at lib/tests/test_kprobes.c:64
>   Expected 0 == register_kprobe(&kp), but
>       register_kprobe(&kp) == -22 (0xffffffffffffffea)
> ...
>   Unable to handle kernel paging request ...

Oops, good catch! Thanks for fixing!

> 
> The testsuite defines several kprobes and kretprobes as static variables
> that are preserved across test runs.
> 
> After register_kprobe and unregister_kprobe, a kprobe contains some
> leftover data that must be cleared before the kprobe can be registered
> again. The tests are setting symbol_name to define the probe location.
> Address and flags must be cleared.
> 
> The existing code clears some of the probes between subsequent tests, but
> not between two test runs. The leftover data from a previous test run
> makes the registrations fail in the next run.
> 
> Move the cleanups for all kprobes into kprobes_test_init, this function
> is called before each single test (including the first test of a test
> run).

Yeah, this looks good to me. Let me pick it.

> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  lib/tests/test_kprobes.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/tests/test_kprobes.c b/lib/tests/test_kprobes.c
> index b7582010125c..06e729e4de05 100644
> --- a/lib/tests/test_kprobes.c
> +++ b/lib/tests/test_kprobes.c
> @@ -12,6 +12,12 @@
>  
>  #define div_factor 3
>  
> +#define KP_CLEAR(_kp) \
> +do { \
> +	(_kp).addr = NULL; \
> +	(_kp).flags = 0; \
> +} while (0)
> +
>  static u32 rand1, preh_val, posth_val;
>  static u32 (*target)(u32 value);
>  static u32 (*recursed_target)(u32 value);
> @@ -125,10 +131,6 @@ static void test_kprobes(struct kunit *test)
>  
>  	current_test = test;
>  
> -	/* addr and flags should be cleard for reusing kprobe. */
> -	kp.addr = NULL;
> -	kp.flags = 0;
> -
>  	KUNIT_EXPECT_EQ(test, 0, register_kprobes(kps, 2));
>  	preh_val = 0;
>  	posth_val = 0;
> @@ -226,9 +228,6 @@ static void test_kretprobes(struct kunit *test)
>  	struct kretprobe *rps[2] = {&rp, &rp2};
>  
>  	current_test = test;
> -	/* addr and flags should be cleard for reusing kprobe. */
> -	rp.kp.addr = NULL;
> -	rp.kp.flags = 0;
>  	KUNIT_EXPECT_EQ(test, 0, register_kretprobes(rps, 2));
>  
>  	krph_val = 0;
> @@ -290,8 +289,6 @@ 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
> @@ -352,8 +349,6 @@ static void test_stacktrace_on_nested_kretprobe(struct kunit *test)
>  	struct kretprobe *rps[2] = {&rp3, &rp4};
>  
>  	current_test = test;
> -	rp3.kp.addr = NULL;
> -	rp3.kp.flags = 0;
>  
>  	//KUNIT_ASSERT_NE(test, myretaddr, stacktrace_driver());
>  
> @@ -367,6 +362,18 @@ static void test_stacktrace_on_nested_kretprobe(struct kunit *test)
>  
>  static int kprobes_test_init(struct kunit *test)
>  {
> +	KP_CLEAR(kp);
> +	KP_CLEAR(kp2);
> +	KP_CLEAR(kp_missed);
> +#ifdef CONFIG_KRETPROBES
> +	KP_CLEAR(rp.kp);
> +	KP_CLEAR(rp2.kp);
> +#ifdef CONFIG_ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
> +	KP_CLEAR(rp3.kp);
> +	KP_CLEAR(rp4.kp);
> +#endif
> +#endif
> +
>  	target = kprobe_target;
>  	target2 = kprobe_target2;
>  	recursed_target = kprobe_recursed_target;
> -- 
> 2.43.7
> 


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