Re: [PATCH v2 4/4] kprobes: Add KUnit test cases for kprobes regressions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] kprobes: test_optprobe_rearm_unoptimizing() misses jump
  overwrite on re-arm
--

commit d9ce37575f42c89c2d4a066b90a06cb4ee50df98
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

kprobes: Add KUnit test cases for kprobes regressions

This commit adds four KUnit test cases to lib/tests/test_kprobes.c covering
regressions in kprobes registration, ftrace ops switching, and optimization
state transitions.

> diff --git a/lib/tests/test_kprobes.c b/lib/tests/test_kprobes.c
> index 06e729e4de051..d692612bc2245 100644
> --- a/lib/tests/test_kprobes.c
> +++ b/lib/tests/test_kprobes.c
[ ... ]
> +/* Test for: kprobes: Do not overwrite jump instruction when re-arming unoptimizing probe */
> +static void test_optprobe_rearm_unoptimizing(struct kunit *test)
> +{
> +	struct kprobe opt_kp = {
> +		.symbol_name = "kprobe_opt_target",
> +		.pre_handler = kp_pre_handler,
> +	};
> +
> +	current_test = test;
> +	KUNIT_ASSERT_EQ(test, 0, register_kprobe(&opt_kp));
> +
> +	/* Allow background optimizer thread to jump-optimize the probe */
> +	wait_for_kprobe_optimizer();
> +
> +	/* Temporarily disable probe (queues on unoptimizing_list, leaves jump) */
> +	KUNIT_EXPECT_EQ(test, 0, disable_kprobe(&opt_kp));
> +
> +	/* Immediately re-enable (re-arm) before optimizer thread unoptimizes */
> +	KUNIT_EXPECT_EQ(test, 0, enable_kprobe(&opt_kp));
> +
> +	/* Verify jump opcode was not corrupted by an unaligned INT3 overwrite */
> +	preh_val = 0;
> +	opt_target(rand1);
> +	KUNIT_EXPECT_NE(test, 0, preh_val);
> +
> +	unregister_kprobe(&opt_kp);
> +}
> +#endif

[Severity: Medium]
Does test_optprobe_rearm_unoptimizing() pass even when enable_kprobe()
overwrites the jump instruction at opt_kp.addr with a breakpoint?

When disable_kprobe() runs on the optimized probe, __disarm_kprobe() queues
it on unoptimizing_list and skips arch_disarm_kprobe(), leaving the jump in
place.  However, enable_kprobe() calls __arm_kprobe(), which unconditionally
calls arch_arm_kprobe() before optimize_kprobe():

kernel/kprobes.c:__arm_kprobe() {
    ...
	arch_arm_kprobe(p);
	optimize_kprobe(p);	/* Try to optimize (add kprobe to a list) */
}

arch_arm_kprobe() overwrites the jump opcode at p->addr with a breakpoint,
while optimize_kprobe() sees KPROBE_FLAG_OPTIMIZED and
optprobe_queued_unopt(op), dequeues the probe from unoptimizing_list, and
returns early without re-optimizing it.  This leaves the probe permanently
de-optimized with a breakpoint at p->addr while KPROBE_FLAG_OPTIMIZED
remains set.  Because hitting that breakpoint still invokes kp_pre_handler(),
checking only that preh_val is non-zero does not detect that the jump opcode
was overwritten.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179427358690.291326.2078291446658598883.stgit@devnote2?part=4