Re: [PATCH v4 2/2] riscv: kprobes: add KUnit test for LR/SC sequence rejection

[Nam Cao <namcao@linutronix.de>]

Xiaofeng Yuan <yuanxiaofeng@eswincomputing.com> writes:
> +SYM_FUNC_START(test_kprobes_lrsc)
> +	li a0, 1
> +1:
> +	lr.w a1, 0(x0)

It would be nice to place this instruction right at the beginning (ditch
the "li a0, 1"), so we can test that it is not wrongly skipped.

> +.Llrsc_inside:

Shouldn't the lr.w instruction be rejected as well?

> +	addi a1, a1, 1
> +	sc.w a2, a1, 0(x0)
> +	bnez a2, 1b
> +.Llrsc_after:
> +	li a0, KPROBE_TEST_MAGIC

We don't need to return KPROBE_TEST_MAGIC for this test case.

> +	ret
> +SYM_FUNC_END(test_kprobes_lrsc)
> +
> +.section .rodata
> +SYM_DATA_START(test_kprobes_lrsc_offsets)
> +	RISCV_PTR .Llrsc_inside - test_kprobes_lrsc
> +	RISCV_PTR .Llrsc_after - test_kprobes_lrsc

Why do we do the subtraction here, only to do the reverse addition in
test_kprobe_lrsc()?

> +SYM_DATA_END(test_kprobes_lrsc_offsets)
> +
>  .section .rodata
>  SYM_DATA_START(test_kprobes_addresses)
>  	RISCV_PTR test_kprobes_add_addr1
> diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.c b/arch/riscv/kernel/tests/kprobes/test-kprobes.c
> index 027424a3ff..25bd103e20 100644
> --- a/arch/riscv/kernel/tests/kprobes/test-kprobes.c
> +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.c
> @@ -43,8 +43,27 @@ static void test_kprobe_riscv(struct kunit *test)
>  	kfree(kp);
>  }
>  
> +static void test_kprobe_lrsc(struct kunit *test)
> +{
> +	struct kprobe kp = {};
> +
> +	kp.pre_handler = kprobe_dummy_handler;
> +
> +	/* a probe inside an LR/SC sequence must be rejected */
> +	kp.addr = (kprobe_opcode_t *)((unsigned long)test_kprobes_lrsc +
> +				      test_kprobes_lrsc_offsets[0]);
> +	KUNIT_EXPECT_LT(test, register_kprobe(&kp), 0);
> +
> +	/* a probe right after the sequence must be accepted */
> +	kp.addr = (kprobe_opcode_t *)((unsigned long)test_kprobes_lrsc +
> +				      test_kprobes_lrsc_offsets[1]);
> +	KUNIT_EXPECT_EQ(test, 0, register_kprobe(&kp));
> +	unregister_kprobe(&kp);
> +}

We should have at least 4 checks: right before, first inside, last
inside, right after.

Nam

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv