[v2] riscv: Add kprobes KUnit test

[PATCH v2] riscv: Add kprobes KUnit test

Posted by Nam Cao 7 months, 1 week ago

Add KUnit test for riscv kprobes, mostly for simulated instructions. The
test install kprobes into multiple sample functions, and check that these
functions still return the expected magic value.

This test can detect some kprobe bugs reported in the past (in Link:).

Link: https://lore.kernel.org/linux-riscv/20241119111056.2554419-1-namcao@linutronix.de/
Link: https://lore.kernel.org/stable/c7e463c0-8cad-4f4e-addd-195c06b7b6de@iscas.ac.cn/
Link: https://lore.kernel.org/linux-riscv/20230829182500.61875-1-namcaov@gmail.com/
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
v2: rewrite the auipc test so that clang can compile

 arch/riscv/kernel/tests/Kconfig.debug         |  12 +
 arch/riscv/kernel/tests/Makefile              |   1 +
 arch/riscv/kernel/tests/kprobes/Makefile      |   1 +
 .../kernel/tests/kprobes/test-kprobes-asm.S   | 229 ++++++++++++++++++
 .../riscv/kernel/tests/kprobes/test-kprobes.c |  56 +++++
 .../riscv/kernel/tests/kprobes/test-kprobes.h |  24 ++
 6 files changed, 323 insertions(+)
 create mode 100644 arch/riscv/kernel/tests/kprobes/Makefile
 create mode 100644 arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S
 create mode 100644 arch/riscv/kernel/tests/kprobes/test-kprobes.c
 create mode 100644 arch/riscv/kernel/tests/kprobes/test-kprobes.h

diff --git a/arch/riscv/kernel/tests/Kconfig.debug b/arch/riscv/kernel/tests/Kconfig.debug
index 78cea5d2c270..5db4df44279e 100644
--- a/arch/riscv/kernel/tests/Kconfig.debug
+++ b/arch/riscv/kernel/tests/Kconfig.debug
@@ -30,6 +30,18 @@ config RISCV_MODULE_LINKING_KUNIT
 
          If unsure, say N.
 
+config RISCV_KPROBES_KUNIT
+       bool "KUnit test for riscv kprobes" if !KUNIT_ALL_TESTS
+       depends on KUNIT
+       depends on KPROBES
+       default KUNIT_ALL_TESTS
+       help
+         Enable testing for riscv kprobes. Useful for riscv and/or kprobes
+         development. The test verifies that kprobes do not change the behaviour
+         of some sample functions.
+
+         If unsure, say N.
+
 endif # RUNTIME_TESTING_MENU
 
 endmenu # "arch/riscv/kernel runtime Testing"
diff --git a/arch/riscv/kernel/tests/Makefile b/arch/riscv/kernel/tests/Makefile
index 7d6c76cffe20..407e7e6c28dc 100644
--- a/arch/riscv/kernel/tests/Makefile
+++ b/arch/riscv/kernel/tests/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_RISCV_MODULE_LINKING_KUNIT)	+= module_test/
+obj-$(CONFIG_RISCV_KPROBES_KUNIT)		+= kprobes/
diff --git a/arch/riscv/kernel/tests/kprobes/Makefile b/arch/riscv/kernel/tests/kprobes/Makefile
new file mode 100644
index 000000000000..4cb6c66a98e8
--- /dev/null
+++ b/arch/riscv/kernel/tests/kprobes/Makefile
@@ -0,0 +1 @@
+obj-y += test-kprobes.o test-kprobes-asm.o
diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S b/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S
new file mode 100644
index 000000000000..b951d0f12482
--- /dev/null
+++ b/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S
@@ -0,0 +1,229 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#include <linux/linkage.h>
+#include <asm/asm.h>
+#include "test-kprobes.h"
+
+SYM_FUNC_START(test_kprobes_add)
+	li a1, KPROBE_TEST_MAGIC_UPPER
+	li a2, KPROBE_TEST_MAGIC_LOWER
+test_kprobes_add_addr1:
+	add a1, a1, a2
+test_kprobes_add_addr2:
+	add a0, a1, x0
+	ret
+SYM_FUNC_END(test_kprobes_add)
+
+SYM_FUNC_START(test_kprobes_jal)
+	li a0, 0
+	mv a1, ra
+	.option push
+	.option norvc
+test_kprobes_jal_addr1:
+	jal x0, 2f
+	ret
+	.option pop
+1:	li a0, KPROBE_TEST_MAGIC_UPPER
+	ret
+	.option push
+	.option norvc
+test_kprobes_jal_addr2:
+2:	jal 1b
+	.option pop
+	li a2, KPROBE_TEST_MAGIC_LOWER
+	add a0, a0, a2
+	jr a1
+SYM_FUNC_END(test_kprobes_jal)
+
+SYM_FUNC_START(test_kprobes_jalr)
+	la a0, 1f
+	mv a1, ra
+	.option push
+	.option norvc
+test_kprobes_jalr_addr:
+	jalr a0
+	.option pop
+	li t0, KPROBE_TEST_MAGIC_UPPER
+	add a0, a0, t0
+	jr a1
+1:	li a0, KPROBE_TEST_MAGIC_LOWER
+	ret
+SYM_FUNC_END(test_kprobes_jalr)
+
+SYM_FUNC_START(test_kprobes_auipc)
+test_kprobes_auipc_addr:
+	auipc a0, KPROBE_TEST_MAGIC_LOWER
+	la a1, test_kprobes_auipc_addr
+	sub a0, a0, a1
+	srli a0, a0, 12
+	li a1, KPROBE_TEST_MAGIC_UPPER
+	add a0, a0, a1
+	ret
+SYM_FUNC_END(test_kprobes_auipc)
+
+SYM_FUNC_START(test_kprobes_branch)
+	.option push
+	.option norvc
+	li a0, 0
+	li a1, 1
+	li a2, 2
+test_kprobes_branch_addr1:
+	beqz a0, 1f
+	ret
+1:
+test_kprobes_branch_addr2:
+	beqz a1, 3f
+test_kprobes_branch_addr3:
+	bnez a0, 3f
+test_kprobes_branch_addr4:
+	bnez a2, 1f
+	ret
+1:
+test_kprobes_branch_addr5:
+	bge a1, a2, 3f
+test_kprobes_branch_addr6:
+	bge a2, a1, 2f
+	ret
+1:
+	li t0, KPROBE_TEST_MAGIC_UPPER
+	add a0, a0, t0
+	ret
+2:
+test_kprobes_branch_addr7:
+	blt a2, a1, 3f
+	li a0, KPROBE_TEST_MAGIC_LOWER
+test_kprobes_branch_addr8:
+	blt a1, a2, 1b
+3:
+	li a0, 0
+	ret
+	.option pop
+SYM_FUNC_END(test_kprobes_branch)
+
+#ifdef CONFIG_RISCV_ISA_C
+
+SYM_FUNC_START(test_kprobes_c_j)
+	li a0, 0
+test_kprobes_branch_c_j_addr1:
+	c.j 2f
+1:
+	li a1, KPROBE_TEST_MAGIC_UPPER
+	add a0, a0, a1
+	ret
+2:	li a0, KPROBE_TEST_MAGIC_LOWER
+test_kprobes_branch_c_j_addr2:
+	c.j 1b
+SYM_FUNC_END(test_kprobes_c_j)
+
+SYM_FUNC_START(test_kprobes_c_jr)
+	la a0, 2f
+test_kprobes_c_jr_addr1:
+	c.jr a0
+	ret
+1:	li a1, KPROBE_TEST_MAGIC_LOWER
+	add a0, a0, a1
+	ret
+2:
+	li a0, KPROBE_TEST_MAGIC_UPPER
+	la a1, 1b
+test_kprobes_c_jr_addr2:
+	c.jr a1
+SYM_FUNC_END(test_kprobes_c_jr)
+
+SYM_FUNC_START(test_kprobes_c_jalr)
+	mv a1, ra
+	la a0, 1f
+test_kprobes_c_jalr_addr:
+	c.jalr a0
+	li a2, KPROBE_TEST_MAGIC_UPPER
+	add a0, a0, a2
+	jr a1
+1:	li a0, KPROBE_TEST_MAGIC_LOWER
+	ret
+SYM_FUNC_END(test_kprobes_c_jalr)
+
+SYM_FUNC_START(test_kprobes_c_beqz)
+	li a0, 0
+	li a1, 1
+test_kprobes_c_beqz_addr1:
+	c.beqz a0, 2f
+	ret
+1:	li a1, KPROBE_TEST_MAGIC_UPPER
+	add a0, a0, a1
+	ret
+test_kprobes_c_beqz_addr2:
+2:	c.beqz a1, 3f
+	li a0, KPROBE_TEST_MAGIC_LOWER
+	mv a1, x0
+test_kprobes_c_beqz_addr3:
+	c.beqz a1, 1b
+3:	li a0, 0
+	ret
+SYM_FUNC_END(test_kprobes_c_beqz)
+
+SYM_FUNC_START(test_kprobes_c_bnez)
+	li a0, 0
+	li a1, 1
+test_kprobes_c_bnez_addr1:
+	c.bnez a1, 2f
+	ret
+1:	li a1, KPROBE_TEST_MAGIC_UPPER
+	add a0, a0, a1
+	ret
+test_kprobes_c_bnez_addr2:
+2:	c.bnez a0, 3f
+	li a0, KPROBE_TEST_MAGIC_LOWER
+test_kprobes_c_bnez_addr3:
+	c.bnez a0, 1b
+3:	li a0, 0
+	ret
+SYM_FUNC_END(test_kprobes_c_bnez)
+
+#endif /* CONFIG_RISCV_ISA_C */
+
+SYM_DATA_START(test_kprobes_addresses)
+	RISCV_PTR test_kprobes_add_addr1
+	RISCV_PTR test_kprobes_add_addr2
+	RISCV_PTR test_kprobes_jal_addr1
+	RISCV_PTR test_kprobes_jal_addr2
+	RISCV_PTR test_kprobes_jalr_addr
+	RISCV_PTR test_kprobes_auipc_addr
+	RISCV_PTR test_kprobes_branch_addr1
+	RISCV_PTR test_kprobes_branch_addr2
+	RISCV_PTR test_kprobes_branch_addr3
+	RISCV_PTR test_kprobes_branch_addr4
+	RISCV_PTR test_kprobes_branch_addr5
+	RISCV_PTR test_kprobes_branch_addr6
+	RISCV_PTR test_kprobes_branch_addr7
+	RISCV_PTR test_kprobes_branch_addr8
+#ifdef CONFIG_RISCV_ISA_C
+	RISCV_PTR test_kprobes_branch_c_j_addr1
+	RISCV_PTR test_kprobes_branch_c_j_addr2
+	RISCV_PTR test_kprobes_c_jr_addr1
+	RISCV_PTR test_kprobes_c_jr_addr2
+	RISCV_PTR test_kprobes_c_jalr_addr
+	RISCV_PTR test_kprobes_c_beqz_addr1
+	RISCV_PTR test_kprobes_c_beqz_addr2
+	RISCV_PTR test_kprobes_c_beqz_addr3
+	RISCV_PTR test_kprobes_c_bnez_addr1
+	RISCV_PTR test_kprobes_c_bnez_addr2
+	RISCV_PTR test_kprobes_c_bnez_addr3
+#endif /* CONFIG_RISCV_ISA_C */
+	RISCV_PTR 0
+SYM_DATA_END(test_kprobes_addresses)
+
+SYM_DATA_START(test_kprobes_functions)
+	RISCV_PTR test_kprobes_add
+	RISCV_PTR test_kprobes_jal
+	RISCV_PTR test_kprobes_jalr
+	RISCV_PTR test_kprobes_auipc
+	RISCV_PTR test_kprobes_branch
+#ifdef CONFIG_RISCV_ISA_C
+	RISCV_PTR test_kprobes_c_j
+	RISCV_PTR test_kprobes_c_jr
+	RISCV_PTR test_kprobes_c_jalr
+	RISCV_PTR test_kprobes_c_beqz
+	RISCV_PTR test_kprobes_c_bnez
+#endif /* CONFIG_RISCV_ISA_C */
+	RISCV_PTR 0
+SYM_DATA_END(test_kprobes_functions)
diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.c b/arch/riscv/kernel/tests/kprobes/test-kprobes.c
new file mode 100644
index 000000000000..6f6cdfbf5a95
--- /dev/null
+++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/kernel.h>
+#include <linux/kprobes.h>
+#include <kunit/test.h>
+#include "test-kprobes.h"
+
+static int kprobe_dummy_handler(struct kprobe *kp, struct pt_regs *regs)
+{
+	return 0;
+}
+
+static void test_kprobe_riscv(struct kunit *test)
+{
+	unsigned int num_kprobe = 0;
+	long (*func)(void);
+	struct kprobe *kp;
+	int i;
+
+	while (test_kprobes_addresses[num_kprobe])
+		num_kprobe++;
+
+	kp = kcalloc(num_kprobe, sizeof(*kp), GFP_KERNEL);
+	KUNIT_EXPECT_TRUE(test, kp);
+	if (!kp)
+		return;
+
+	for (i = 0; i < num_kprobe; ++i) {
+		kp[i].addr = test_kprobes_addresses[i];
+		kp[i].pre_handler = kprobe_dummy_handler;
+		KUNIT_EXPECT_EQ(test, 0, register_kprobe(&kp[i]));
+	}
+
+	for (i = 0;; ++i) {
+		func = test_kprobes_functions[i];
+		if (!func)
+			break;
+		KUNIT_EXPECT_EQ_MSG(test, KPROBE_TEST_MAGIC, func(), "function %d broken", i);
+	}
+
+	for (i = 0; i < num_kprobe; ++i)
+		unregister_kprobe(&kp[i]);
+	kfree(kp);
+}
+
+static struct kunit_case kprobes_testcases[] = {
+	KUNIT_CASE(test_kprobe_riscv),
+	{}
+};
+
+static struct kunit_suite kprobes_test_suite = {
+	.name = "kprobes_test_riscv",
+	.test_cases = kprobes_testcases,
+};
+
+kunit_test_suites(&kprobes_test_suite);
diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.h b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
new file mode 100644
index 000000000000..3886ab491ecb
--- /dev/null
+++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef TEST_KPROBES_H
+#define TEST_KPROBES_H
+
+/*
+ * The magic value that all the functions in the test_kprobes_functions array return. The test
+ * installs kprobes into these functions, and verify that the functions still correctly return this
+ * value.
+ */
+#define KPROBE_TEST_MAGIC          0xcafebabe
+#define KPROBE_TEST_MAGIC_LOWER    0x0000babe
+#define KPROBE_TEST_MAGIC_UPPER    0xcafe0000
+
+#ifndef __ASSEMBLY__
+
+/* array of addresses to install kprobes */
+extern void *test_kprobes_addresses[];
+
+/* array of functions that return KPROBE_TEST_MAGIC */
+extern long (*test_kprobes_functions[])(void);
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* TEST_KPROBES_H */
-- 
2.39.5

Re: [PATCH v2] riscv: Add kprobes KUnit test

Posted by Thomas Huth 2 months, 3 weeks ago

On 13/05/2025 17.16, Nam Cao wrote:
> Add KUnit test for riscv kprobes, mostly for simulated instructions. The
> test install kprobes into multiple sample functions, and check that these
> functions still return the expected magic value.
> 
> This test can detect some kprobe bugs reported in the past (in Link:).
> 
> Link: https://lore.kernel.org/linux-riscv/20241119111056.2554419-1-namcao@linutronix.de/
> Link: https://lore.kernel.org/stable/c7e463c0-8cad-4f4e-addd-195c06b7b6de@iscas.ac.cn/
> Link: https://lore.kernel.org/linux-riscv/20230829182500.61875-1-namcaov@gmail.com/
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
...
> diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.h b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
> new file mode 100644
> index 000000000000..3886ab491ecb
> --- /dev/null
> +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +#ifndef TEST_KPROBES_H
> +#define TEST_KPROBES_H
> +
> +/*
> + * The magic value that all the functions in the test_kprobes_functions array return. The test
> + * installs kprobes into these functions, and verify that the functions still correctly return this
> + * value.
> + */
> +#define KPROBE_TEST_MAGIC          0xcafebabe
> +#define KPROBE_TEST_MAGIC_LOWER    0x0000babe
> +#define KPROBE_TEST_MAGIC_UPPER    0xcafe0000
> +
> +#ifndef __ASSEMBLY__

Could you maybe change that into "__ASSEMBLER__" instead of "__ASSEMBLY__" ? 
I'm currently trying to get rid of the latter in the kernel sources, see: 
https://lore.kernel.org/all/20250606070952.498274-1-thuth@redhat.com/

> +/* array of addresses to install kprobes */
> +extern void *test_kprobes_addresses[];
> +
> +/* array of functions that return KPROBE_TEST_MAGIC */
> +extern long (*test_kprobes_functions[])(void);
> +
> +#endif /* __ASSEMBLY__ */

dito.

  Thanks,
   Thomas

Re: [PATCH v2] riscv: Add kprobes KUnit test

Posted by Nam Cao 2 months, 3 weeks ago

Thomas Huth <thuth@redhat.com> writes:

> On 13/05/2025 17.16, Nam Cao wrote:
>> Add KUnit test for riscv kprobes, mostly for simulated instructions. The
>> test install kprobes into multiple sample functions, and check that these
>> functions still return the expected magic value.
>> 
>> This test can detect some kprobe bugs reported in the past (in Link:).
>> 
>> Link: https://lore.kernel.org/linux-riscv/20241119111056.2554419-1-namcao@linutronix.de/
>> Link: https://lore.kernel.org/stable/c7e463c0-8cad-4f4e-addd-195c06b7b6de@iscas.ac.cn/
>> Link: https://lore.kernel.org/linux-riscv/20230829182500.61875-1-namcaov@gmail.com/
>> Signed-off-by: Nam Cao <namcao@linutronix.de>
>> ---
> ...
>> diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.h b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
>> new file mode 100644
>> index 000000000000..3886ab491ecb
>> --- /dev/null
>> +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
>> @@ -0,0 +1,24 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +#ifndef TEST_KPROBES_H
>> +#define TEST_KPROBES_H
>> +
>> +/*
>> + * The magic value that all the functions in the test_kprobes_functions array return. The test
>> + * installs kprobes into these functions, and verify that the functions still correctly return this
>> + * value.
>> + */
>> +#define KPROBE_TEST_MAGIC          0xcafebabe
>> +#define KPROBE_TEST_MAGIC_LOWER    0x0000babe
>> +#define KPROBE_TEST_MAGIC_UPPER    0xcafe0000
>> +
>> +#ifndef __ASSEMBLY__
>
> Could you maybe change that into "__ASSEMBLER__" instead of "__ASSEMBLY__" ? 
> I'm currently trying to get rid of the latter in the kernel sources, see: 
> https://lore.kernel.org/all/20250606070952.498274-1-thuth@redhat.com/

It's been applied, it's up to riscv's maintainers how we should do this.

I can send v3, or a follow-up patch.

Or riscv maintainers can also squash that change into this patch, or
into your patch.

I'm fine with any options.

Nam

Re: [PATCH v2] riscv: Add kprobes KUnit test

Posted by Nam Cao 2 months, 2 weeks ago

Nam Cao <namcao@linutronix.de> writes:
> Thomas Huth <thuth@redhat.com> writes:
>> Could you maybe change that into "__ASSEMBLER__" instead of "__ASSEMBLY__" ? 
>> I'm currently trying to get rid of the latter in the kernel sources, see: 
>> https://lore.kernel.org/all/20250606070952.498274-1-thuth@redhat.com/
>
> It's been applied, it's up to riscv's maintainers how we should do this.
>
> I can send v3, or a follow-up patch.
>
> Or riscv maintainers can also squash that change into this patch, or
> into your patch.
>
> I'm fine with any options.

Riscv pull request is already created. A follow-up patch it is then.

Nam

Re: [PATCH v2] riscv: Add kprobes KUnit test

Posted by Paul Walmsley 2 months, 1 week ago

On Mon, 29 Sep 2025, Nam Cao wrote:

> Nam Cao <namcao@linutronix.de> writes:
> > Thomas Huth <thuth@redhat.com> writes:
> >> Could you maybe change that into "__ASSEMBLER__" instead of "__ASSEMBLY__" ? 
> >> I'm currently trying to get rid of the latter in the kernel sources, see: 
> >> https://lore.kernel.org/all/20250606070952.498274-1-thuth@redhat.com/
> >
> > It's been applied, it's up to riscv's maintainers how we should do this.
> >
> > I can send v3, or a follow-up patch.
> >
> > Or riscv maintainers can also squash that change into this patch, or
> > into your patch.
> >
> > I'm fine with any options.
> 
> Riscv pull request is already created. A follow-up patch it is then.

I've queued the following for v6.18-rc. 

Thomas: have you considered updating checkpatch to scan for instances of 
__ASSEMBLY__?  Might preempt these sorts of manual fixes going forward.


- Paul

From: Paul Walmsley <pjw@kernel.org>
Date: Fri, 10 Oct 2025 15:50:24 -0600
Subject: [PATCH] riscv: kprobes: convert one final __ASSEMBLY__ to
 __ASSEMBLER__

Per the reasoning in commit f811f58597ac ("riscv: Replace __ASSEMBLY__
with __ASSEMBLER__ in non-uapi headers"), convert one last remaining
instance of __ASSEMBLY__ in the arch/riscv kprobes code.  This entered
the tree from patches that were sent before Thomas' changes; and when
I reviewed the kprobes patches before queuing them, I missed this
instance.

Cc: Nam Cao <namcao@linutronix.dev>
Cc: Thomas Huth <thuth@redhat.com>
Link: https://lore.kernel.org/linux-riscv/16b74b63-f223-4f0b-b6e5-31cea5e620b4@redhat.com/
Link: https://lore.kernel.org/linux-riscv/20250606070952.498274-1-thuth@redhat.com/
Signed-off-by: Paul Walmsley <pjw@kernel.org>
---
 arch/riscv/kernel/tests/kprobes/test-kprobes.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.h b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
index 3886ab491ecb..537f44aa9d3f 100644
--- a/arch/riscv/kernel/tests/kprobes/test-kprobes.h
+++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
@@ -11,7 +11,7 @@
 #define KPROBE_TEST_MAGIC_LOWER    0x0000babe
 #define KPROBE_TEST_MAGIC_UPPER    0xcafe0000
 
-#ifndef __ASSEMBLY__
+#ifndef __ASSEMBLER__
 
 /* array of addresses to install kprobes */
 extern void *test_kprobes_addresses[];
@@ -19,6 +19,6 @@ extern void *test_kprobes_addresses[];
 /* array of functions that return KPROBE_TEST_MAGIC */
 extern long (*test_kprobes_functions[])(void);
 
-#endif /* __ASSEMBLY__ */
+#endif /* __ASSEMBLER__ */
 
 #endif /* TEST_KPROBES_H */
-- 
2.48.1

Re: [PATCH v2] riscv: Add kprobes KUnit test

Posted by Thomas Huth 2 months ago

On 11/10/2025 00.03, Paul Walmsley wrote:
> On Mon, 29 Sep 2025, Nam Cao wrote:
> 
>> Nam Cao <namcao@linutronix.de> writes:
>>> Thomas Huth <thuth@redhat.com> writes:
>>>> Could you maybe change that into "__ASSEMBLER__" instead of "__ASSEMBLY__" ?
>>>> I'm currently trying to get rid of the latter in the kernel sources, see:
>>>> https://lore.kernel.org/all/20250606070952.498274-1-thuth@redhat.com/
>>>
>>> It's been applied, it's up to riscv's maintainers how we should do this.
>>>
>>> I can send v3, or a follow-up patch.
>>>
>>> Or riscv maintainers can also squash that change into this patch, or
>>> into your patch.
>>>
>>> I'm fine with any options.
>>
>> Riscv pull request is already created. A follow-up patch it is then.
> 
> I've queued the following for v6.18-rc.

Thanks a lot!

> Thomas: have you considered updating checkpatch to scan for instances of
> __ASSEMBLY__?  Might preempt these sorts of manual fixes going forward.

I hope to get in the final patches soon ... so once that's done, 
__ASSEMBLY__ won't get defined anymore, so using it by accident will then 
cause a build error in assembly code - I hope that's obvious enough that we 
don't need a (temporary) patch for checkpatch.pl.

  Thomas


> 
> From: Paul Walmsley <pjw@kernel.org>
> Date: Fri, 10 Oct 2025 15:50:24 -0600
> Subject: [PATCH] riscv: kprobes: convert one final __ASSEMBLY__ to
>   __ASSEMBLER__
> 
> Per the reasoning in commit f811f58597ac ("riscv: Replace __ASSEMBLY__
> with __ASSEMBLER__ in non-uapi headers"), convert one last remaining
> instance of __ASSEMBLY__ in the arch/riscv kprobes code.  This entered
> the tree from patches that were sent before Thomas' changes; and when
> I reviewed the kprobes patches before queuing them, I missed this
> instance.
> 
> Cc: Nam Cao <namcao@linutronix.dev>
> Cc: Thomas Huth <thuth@redhat.com>
> Link: https://lore.kernel.org/linux-riscv/16b74b63-f223-4f0b-b6e5-31cea5e620b4@redhat.com/
> Link: https://lore.kernel.org/linux-riscv/20250606070952.498274-1-thuth@redhat.com/
> Signed-off-by: Paul Walmsley <pjw@kernel.org>
> ---
>   arch/riscv/kernel/tests/kprobes/test-kprobes.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.h b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
> index 3886ab491ecb..537f44aa9d3f 100644
> --- a/arch/riscv/kernel/tests/kprobes/test-kprobes.h
> +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
> @@ -11,7 +11,7 @@
>   #define KPROBE_TEST_MAGIC_LOWER    0x0000babe
>   #define KPROBE_TEST_MAGIC_UPPER    0xcafe0000
>   
> -#ifndef __ASSEMBLY__
> +#ifndef __ASSEMBLER__
>   
>   /* array of addresses to install kprobes */
>   extern void *test_kprobes_addresses[];
> @@ -19,6 +19,6 @@ extern void *test_kprobes_addresses[];
>   /* array of functions that return KPROBE_TEST_MAGIC */
>   extern long (*test_kprobes_functions[])(void);
>   
> -#endif /* __ASSEMBLY__ */
> +#endif /* __ASSEMBLER__ */
>   
>   #endif /* TEST_KPROBES_H */

Re: [PATCH v2] riscv: Add kprobes KUnit test

Posted by Alexandre Ghiti 7 months ago

On 13/05/2025 17:16, Nam Cao wrote:
> Add KUnit test for riscv kprobes, mostly for simulated instructions. The
> test install kprobes into multiple sample functions, and check that these
> functions still return the expected magic value.
>
> This test can detect some kprobe bugs reported in the past (in Link:).
>
> Link: https://lore.kernel.org/linux-riscv/20241119111056.2554419-1-namcao@linutronix.de/
> Link: https://lore.kernel.org/stable/c7e463c0-8cad-4f4e-addd-195c06b7b6de@iscas.ac.cn/
> Link: https://lore.kernel.org/linux-riscv/20230829182500.61875-1-namcaov@gmail.com/
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
> v2: rewrite the auipc test so that clang can compile
>
>   arch/riscv/kernel/tests/Kconfig.debug         |  12 +
>   arch/riscv/kernel/tests/Makefile              |   1 +
>   arch/riscv/kernel/tests/kprobes/Makefile      |   1 +
>   .../kernel/tests/kprobes/test-kprobes-asm.S   | 229 ++++++++++++++++++
>   .../riscv/kernel/tests/kprobes/test-kprobes.c |  56 +++++
>   .../riscv/kernel/tests/kprobes/test-kprobes.h |  24 ++
>   6 files changed, 323 insertions(+)
>   create mode 100644 arch/riscv/kernel/tests/kprobes/Makefile
>   create mode 100644 arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S
>   create mode 100644 arch/riscv/kernel/tests/kprobes/test-kprobes.c
>   create mode 100644 arch/riscv/kernel/tests/kprobes/test-kprobes.h
>
> diff --git a/arch/riscv/kernel/tests/Kconfig.debug b/arch/riscv/kernel/tests/Kconfig.debug
> index 78cea5d2c270..5db4df44279e 100644
> --- a/arch/riscv/kernel/tests/Kconfig.debug
> +++ b/arch/riscv/kernel/tests/Kconfig.debug
> @@ -30,6 +30,18 @@ config RISCV_MODULE_LINKING_KUNIT
>   
>            If unsure, say N.
>   
> +config RISCV_KPROBES_KUNIT
> +       bool "KUnit test for riscv kprobes" if !KUNIT_ALL_TESTS
> +       depends on KUNIT
> +       depends on KPROBES
> +       default KUNIT_ALL_TESTS
> +       help
> +         Enable testing for riscv kprobes. Useful for riscv and/or kprobes
> +         development. The test verifies that kprobes do not change the behaviour
> +         of some sample functions.
> +
> +         If unsure, say N.
> +
>   endif # RUNTIME_TESTING_MENU
>   
>   endmenu # "arch/riscv/kernel runtime Testing"
> diff --git a/arch/riscv/kernel/tests/Makefile b/arch/riscv/kernel/tests/Makefile
> index 7d6c76cffe20..407e7e6c28dc 100644
> --- a/arch/riscv/kernel/tests/Makefile
> +++ b/arch/riscv/kernel/tests/Makefile
> @@ -1 +1,2 @@
>   obj-$(CONFIG_RISCV_MODULE_LINKING_KUNIT)	+= module_test/
> +obj-$(CONFIG_RISCV_KPROBES_KUNIT)		+= kprobes/
> diff --git a/arch/riscv/kernel/tests/kprobes/Makefile b/arch/riscv/kernel/tests/kprobes/Makefile
> new file mode 100644
> index 000000000000..4cb6c66a98e8
> --- /dev/null
> +++ b/arch/riscv/kernel/tests/kprobes/Makefile
> @@ -0,0 +1 @@
> +obj-y += test-kprobes.o test-kprobes-asm.o
> diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S b/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S
> new file mode 100644
> index 000000000000..b951d0f12482
> --- /dev/null
> +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes-asm.S
> @@ -0,0 +1,229 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#include <linux/linkage.h>
> +#include <asm/asm.h>
> +#include "test-kprobes.h"
> +
> +SYM_FUNC_START(test_kprobes_add)
> +	li a1, KPROBE_TEST_MAGIC_UPPER
> +	li a2, KPROBE_TEST_MAGIC_LOWER
> +test_kprobes_add_addr1:
> +	add a1, a1, a2
> +test_kprobes_add_addr2:
> +	add a0, a1, x0
> +	ret
> +SYM_FUNC_END(test_kprobes_add)
> +
> +SYM_FUNC_START(test_kprobes_jal)
> +	li a0, 0
> +	mv a1, ra
> +	.option push
> +	.option norvc
> +test_kprobes_jal_addr1:
> +	jal x0, 2f
> +	ret
> +	.option pop
> +1:	li a0, KPROBE_TEST_MAGIC_UPPER
> +	ret
> +	.option push
> +	.option norvc
> +test_kprobes_jal_addr2:
> +2:	jal 1b
> +	.option pop
> +	li a2, KPROBE_TEST_MAGIC_LOWER
> +	add a0, a0, a2
> +	jr a1
> +SYM_FUNC_END(test_kprobes_jal)
> +
> +SYM_FUNC_START(test_kprobes_jalr)
> +	la a0, 1f
> +	mv a1, ra
> +	.option push
> +	.option norvc
> +test_kprobes_jalr_addr:
> +	jalr a0
> +	.option pop
> +	li t0, KPROBE_TEST_MAGIC_UPPER
> +	add a0, a0, t0
> +	jr a1
> +1:	li a0, KPROBE_TEST_MAGIC_LOWER
> +	ret
> +SYM_FUNC_END(test_kprobes_jalr)
> +
> +SYM_FUNC_START(test_kprobes_auipc)
> +test_kprobes_auipc_addr:
> +	auipc a0, KPROBE_TEST_MAGIC_LOWER
> +	la a1, test_kprobes_auipc_addr
> +	sub a0, a0, a1
> +	srli a0, a0, 12
> +	li a1, KPROBE_TEST_MAGIC_UPPER
> +	add a0, a0, a1
> +	ret
> +SYM_FUNC_END(test_kprobes_auipc)
> +
> +SYM_FUNC_START(test_kprobes_branch)
> +	.option push
> +	.option norvc
> +	li a0, 0
> +	li a1, 1
> +	li a2, 2
> +test_kprobes_branch_addr1:
> +	beqz a0, 1f
> +	ret
> +1:
> +test_kprobes_branch_addr2:
> +	beqz a1, 3f
> +test_kprobes_branch_addr3:
> +	bnez a0, 3f
> +test_kprobes_branch_addr4:
> +	bnez a2, 1f
> +	ret
> +1:
> +test_kprobes_branch_addr5:
> +	bge a1, a2, 3f
> +test_kprobes_branch_addr6:
> +	bge a2, a1, 2f
> +	ret
> +1:
> +	li t0, KPROBE_TEST_MAGIC_UPPER
> +	add a0, a0, t0
> +	ret
> +2:
> +test_kprobes_branch_addr7:
> +	blt a2, a1, 3f
> +	li a0, KPROBE_TEST_MAGIC_LOWER
> +test_kprobes_branch_addr8:
> +	blt a1, a2, 1b
> +3:
> +	li a0, 0
> +	ret
> +	.option pop
> +SYM_FUNC_END(test_kprobes_branch)
> +
> +#ifdef CONFIG_RISCV_ISA_C
> +
> +SYM_FUNC_START(test_kprobes_c_j)
> +	li a0, 0
> +test_kprobes_branch_c_j_addr1:
> +	c.j 2f
> +1:
> +	li a1, KPROBE_TEST_MAGIC_UPPER
> +	add a0, a0, a1
> +	ret
> +2:	li a0, KPROBE_TEST_MAGIC_LOWER
> +test_kprobes_branch_c_j_addr2:
> +	c.j 1b
> +SYM_FUNC_END(test_kprobes_c_j)
> +
> +SYM_FUNC_START(test_kprobes_c_jr)
> +	la a0, 2f
> +test_kprobes_c_jr_addr1:
> +	c.jr a0
> +	ret
> +1:	li a1, KPROBE_TEST_MAGIC_LOWER
> +	add a0, a0, a1
> +	ret
> +2:
> +	li a0, KPROBE_TEST_MAGIC_UPPER
> +	la a1, 1b
> +test_kprobes_c_jr_addr2:
> +	c.jr a1
> +SYM_FUNC_END(test_kprobes_c_jr)
> +
> +SYM_FUNC_START(test_kprobes_c_jalr)
> +	mv a1, ra
> +	la a0, 1f
> +test_kprobes_c_jalr_addr:
> +	c.jalr a0
> +	li a2, KPROBE_TEST_MAGIC_UPPER
> +	add a0, a0, a2
> +	jr a1
> +1:	li a0, KPROBE_TEST_MAGIC_LOWER
> +	ret
> +SYM_FUNC_END(test_kprobes_c_jalr)
> +
> +SYM_FUNC_START(test_kprobes_c_beqz)
> +	li a0, 0
> +	li a1, 1
> +test_kprobes_c_beqz_addr1:
> +	c.beqz a0, 2f
> +	ret
> +1:	li a1, KPROBE_TEST_MAGIC_UPPER
> +	add a0, a0, a1
> +	ret
> +test_kprobes_c_beqz_addr2:
> +2:	c.beqz a1, 3f
> +	li a0, KPROBE_TEST_MAGIC_LOWER
> +	mv a1, x0
> +test_kprobes_c_beqz_addr3:
> +	c.beqz a1, 1b
> +3:	li a0, 0
> +	ret
> +SYM_FUNC_END(test_kprobes_c_beqz)
> +
> +SYM_FUNC_START(test_kprobes_c_bnez)
> +	li a0, 0
> +	li a1, 1
> +test_kprobes_c_bnez_addr1:
> +	c.bnez a1, 2f
> +	ret
> +1:	li a1, KPROBE_TEST_MAGIC_UPPER
> +	add a0, a0, a1
> +	ret
> +test_kprobes_c_bnez_addr2:
> +2:	c.bnez a0, 3f
> +	li a0, KPROBE_TEST_MAGIC_LOWER
> +test_kprobes_c_bnez_addr3:
> +	c.bnez a0, 1b
> +3:	li a0, 0
> +	ret
> +SYM_FUNC_END(test_kprobes_c_bnez)
> +
> +#endif /* CONFIG_RISCV_ISA_C */
> +
> +SYM_DATA_START(test_kprobes_addresses)
> +	RISCV_PTR test_kprobes_add_addr1
> +	RISCV_PTR test_kprobes_add_addr2
> +	RISCV_PTR test_kprobes_jal_addr1
> +	RISCV_PTR test_kprobes_jal_addr2
> +	RISCV_PTR test_kprobes_jalr_addr
> +	RISCV_PTR test_kprobes_auipc_addr
> +	RISCV_PTR test_kprobes_branch_addr1
> +	RISCV_PTR test_kprobes_branch_addr2
> +	RISCV_PTR test_kprobes_branch_addr3
> +	RISCV_PTR test_kprobes_branch_addr4
> +	RISCV_PTR test_kprobes_branch_addr5
> +	RISCV_PTR test_kprobes_branch_addr6
> +	RISCV_PTR test_kprobes_branch_addr7
> +	RISCV_PTR test_kprobes_branch_addr8
> +#ifdef CONFIG_RISCV_ISA_C
> +	RISCV_PTR test_kprobes_branch_c_j_addr1
> +	RISCV_PTR test_kprobes_branch_c_j_addr2
> +	RISCV_PTR test_kprobes_c_jr_addr1
> +	RISCV_PTR test_kprobes_c_jr_addr2
> +	RISCV_PTR test_kprobes_c_jalr_addr
> +	RISCV_PTR test_kprobes_c_beqz_addr1
> +	RISCV_PTR test_kprobes_c_beqz_addr2
> +	RISCV_PTR test_kprobes_c_beqz_addr3
> +	RISCV_PTR test_kprobes_c_bnez_addr1
> +	RISCV_PTR test_kprobes_c_bnez_addr2
> +	RISCV_PTR test_kprobes_c_bnez_addr3
> +#endif /* CONFIG_RISCV_ISA_C */
> +	RISCV_PTR 0
> +SYM_DATA_END(test_kprobes_addresses)
> +
> +SYM_DATA_START(test_kprobes_functions)
> +	RISCV_PTR test_kprobes_add
> +	RISCV_PTR test_kprobes_jal
> +	RISCV_PTR test_kprobes_jalr
> +	RISCV_PTR test_kprobes_auipc
> +	RISCV_PTR test_kprobes_branch
> +#ifdef CONFIG_RISCV_ISA_C
> +	RISCV_PTR test_kprobes_c_j
> +	RISCV_PTR test_kprobes_c_jr
> +	RISCV_PTR test_kprobes_c_jalr
> +	RISCV_PTR test_kprobes_c_beqz
> +	RISCV_PTR test_kprobes_c_bnez
> +#endif /* CONFIG_RISCV_ISA_C */
> +	RISCV_PTR 0
> +SYM_DATA_END(test_kprobes_functions)
> diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.c b/arch/riscv/kernel/tests/kprobes/test-kprobes.c
> new file mode 100644
> index 000000000000..6f6cdfbf5a95
> --- /dev/null
> +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.c
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <linux/kernel.h>
> +#include <linux/kprobes.h>
> +#include <kunit/test.h>
> +#include "test-kprobes.h"
> +
> +static int kprobe_dummy_handler(struct kprobe *kp, struct pt_regs *regs)
> +{
> +	return 0;
> +}
> +
> +static void test_kprobe_riscv(struct kunit *test)
> +{
> +	unsigned int num_kprobe = 0;
> +	long (*func)(void);
> +	struct kprobe *kp;
> +	int i;
> +
> +	while (test_kprobes_addresses[num_kprobe])
> +		num_kprobe++;
> +
> +	kp = kcalloc(num_kprobe, sizeof(*kp), GFP_KERNEL);
> +	KUNIT_EXPECT_TRUE(test, kp);
> +	if (!kp)
> +		return;
> +
> +	for (i = 0; i < num_kprobe; ++i) {
> +		kp[i].addr = test_kprobes_addresses[i];
> +		kp[i].pre_handler = kprobe_dummy_handler;
> +		KUNIT_EXPECT_EQ(test, 0, register_kprobe(&kp[i]));


Will kp be freed if the test fails?


> +	}
> +
> +	for (i = 0;; ++i) {
> +		func = test_kprobes_functions[i];
> +		if (!func)
> +			break;
> +		KUNIT_EXPECT_EQ_MSG(test, KPROBE_TEST_MAGIC, func(), "function %d broken", i);
> +	}
> +
> +	for (i = 0; i < num_kprobe; ++i)
> +		unregister_kprobe(&kp[i]);
> +	kfree(kp);
> +}
> +
> +static struct kunit_case kprobes_testcases[] = {
> +	KUNIT_CASE(test_kprobe_riscv),
> +	{}
> +};
> +
> +static struct kunit_suite kprobes_test_suite = {
> +	.name = "kprobes_test_riscv",
> +	.test_cases = kprobes_testcases,
> +};
> +
> +kunit_test_suites(&kprobes_test_suite);
> diff --git a/arch/riscv/kernel/tests/kprobes/test-kprobes.h b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
> new file mode 100644
> index 000000000000..3886ab491ecb
> --- /dev/null
> +++ b/arch/riscv/kernel/tests/kprobes/test-kprobes.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +#ifndef TEST_KPROBES_H
> +#define TEST_KPROBES_H
> +
> +/*
> + * The magic value that all the functions in the test_kprobes_functions array return. The test
> + * installs kprobes into these functions, and verify that the functions still correctly return this
> + * value.
> + */
> +#define KPROBE_TEST_MAGIC          0xcafebabe
> +#define KPROBE_TEST_MAGIC_LOWER    0x0000babe
> +#define KPROBE_TEST_MAGIC_UPPER    0xcafe0000
> +
> +#ifndef __ASSEMBLY__
> +
> +/* array of addresses to install kprobes */
> +extern void *test_kprobes_addresses[];
> +
> +/* array of functions that return KPROBE_TEST_MAGIC */
> +extern long (*test_kprobes_functions[])(void);
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* TEST_KPROBES_H */


Other than that, you can add:

Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>

I'll merge it for 6.16.

Thanks for adding new tests, KUnit tests do not run yet in the CI but 
I'll add them soon!

Alex

Re: [PATCH v2] riscv: Add kprobes KUnit test

Posted by Nam Cao 7 months ago

On Wed, May 14, 2025 at 10:22:03AM +0200, Alexandre Ghiti wrote:
> On 13/05/2025 17:16, Nam Cao wrote:
> > +	for (i = 0; i < num_kprobe; ++i) {
> > +		kp[i].addr = test_kprobes_addresses[i];
> > +		kp[i].pre_handler = kprobe_dummy_handler;
> > +		KUNIT_EXPECT_EQ(test, 0, register_kprobe(&kp[i]));
> 
> 
> Will kp be freed if the test fails?

Yes. Unlike KUNIT_ASSERT_EQ() which terminates the test, KUNIT_EXPECT_EQ()
still lets the test continues.

> Thanks for adding new tests, KUnit tests do not run yet in the CI but I'll
> add them soon!

Well I promised Björn this test back in 2023 ;) But I wasn't sure how to
implement it (I was still a student at that time) and then got busy with
other stuffs. But I guess better late than never.

Nam