public inbox for linux-riscv@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/1] Support kCFI + BPF on riscv64
@ 2024-03-03 17:02 Puranjay Mohan
  2024-03-03 17:02 ` [PATCH bpf-next 1/1] riscv64/cfi,bpf: " Puranjay Mohan
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Puranjay Mohan @ 2024-03-03 17:02 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Luke Nelson,
	Xi Wang, Björn Töpel, Sami Tolvanen, Peter Zijlstra,
	Kees Cook, linux-riscv, linux-kernel, bpf
  Cc: puranjay12

With CONFIG_CFI_CLANG, the compiler injects a type preamble immediately
before each function and a check to validate the target function type
before indirect calls:

  ; type preamble
    .word <id>
  function:
    ...
  ; indirect call check
    lw      t1, -4(a0)
    lui     t2, <hi20>
    addiw   t2, t2, <lo12>
    beq     t1, t2, .Ltmp0
    ebreak
  .Ltmp0:
    jarl    a0

BPF JIT currently doesn't emit this preamble before BPF programs and when
the calling fuction tries to load the type id from the preamble, it finds
an invalid value there.

This will cause CFI failures like in the following bpf selftest:

root@rv-selftester:~/bpf# ./test_progs -a "rbtree_success"

 CFI failure at bpf_rbtree_add_impl+0x148/0x350 (target: bpf_prog_fb8b097ab47d164a_less+0x0/0x42; expected type: 0x00000000)
 WARNING: CPU: 1 PID: 278 at bpf_rbtree_add_impl+0x148/0x350
 Modules linked in: bpf_testmod(OE) drm fuse dm_mod backlight i2c_core configfs drm_panel_orientation_quirks ip_tables x_tables
 CPU: 1 PID: 278 Comm: test_progs Tainted: P           OE      6.8.0-rc1 #1
 Hardware name: riscv-virtio,qemu (DT)
 epc : bpf_rbtree_add_impl+0x148/0x350
  ra : bpf_prog_27b36e47d273751e_rbtree_first_and_remove+0x1aa/0x35e
 epc : ffffffff805acc0c ra : ffffffff780077fa sp : ff2000000110b9d0
  gp : ffffffff868d6218 tp : ff60000085772a40 t0 : ffffffff86849660
  t1 : 0000000000000000 t2 : ffffffff9e4709a9 s0 : ff2000000110ba50
  s1 : ff60000089c14958 a0 : ff60000089c14758 a1 : ff60000089c14958
  a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
  a5 : 0000000000000000 a6 : ff6000008aba4b30 a7 : ffffffff86849640
  s2 : ff6000008aba4b30 s3 : ff60000089c14758 s4 : ffffffff780079f0
  s5 : 0000000000000000 s6 : ffffffff84c01080 s7 : ff6000008aba4b30
  s8 : 0000000000000000 s9 : 0000000000000000 s10: 0000000000000001
  s11: 0000000000000000 t3 : ffffffff868499e0 t4 : ffffffff868499c0
  t5 : ffffffff86849840 t6 : ffffffff86849860
 status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
 [<ffffffff805acc0c>] bpf_rbtree_add_impl+0x148/0x350
 [<ffffffff780077fa>] bpf_prog_27b36e47d273751e_rbtree_first_and_remove+0x1aa/0x35e
 [<ffffffff8294f32c>] bpf_test_run+0x2a4/0xa3c
 [<ffffffff8294d032>] bpf_prog_test_run_skb+0x47a/0xe52
 [<ffffffff805083ee>] bpf_prog_test_run+0x170/0x548
 [<ffffffff805029c8>] __sys_bpf+0x2d2/0x378
 [<ffffffff804ff570>] __riscv_sys_bpf+0x5c/0x120
 [<ffffffff8000e8fe>] syscall_handler+0x62/0xe4
 [<ffffffff83362df6>] do_trap_ecall_u+0xc6/0x27c
 [<ffffffff833822c4>] ret_from_exception+0x0/0x64
 ---[ end trace 0000000000000000 ]---

The calling function tries to load the type id hash from target_func - 4.
If this memory address is not mapped then it can cause a page fault and
crash the kernel.

This behaviour can be seen by running the 'dummy_st_ops' selftest:

root@rv-selftester:~/bpf# ./test_progs -a dummy_st_ops

 Unable to handle kernel paging request at virtual address ffffffff78204ffc
 Oops [#1]
 Modules linked in: bpf_testmod(OE) drm fuse backlight i2c_core drm_panel_orientation_quirks dm_mod configfs ip_tables x_tables [last unloaded: bpf_testmod(OE)]
 CPU: 3 PID: 356 Comm: test_progs Tainted: P           OE      6.8.0-rc1 #1
 Hardware name: riscv-virtio,qemu (DT)
 epc : bpf_struct_ops_test_run+0x28c/0x5fc
  ra : bpf_struct_ops_test_run+0x26c/0x5fc
 epc : ffffffff82958010 ra : ffffffff82957ff0 sp : ff200000007abc80
  gp : ffffffff868d6218 tp : ff6000008d87b840 t0 : 000000000000000f
  t1 : 0000000000000000 t2 : 000000002005793e s0 : ff200000007abcf0
  s1 : ff6000008a90fee0 a0 : 0000000000000000 a1 : 0000000000000000
  a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
  a5 : ffffffff868dba26 a6 : 0000000000000001 a7 : 0000000052464e43
  s2 : 00007ffffc0a95f0 s3 : ff6000008a90fe80 s4 : ff60000084c24c00
  s5 : ffffffff78205000 s6 : ff60000088750648 s7 : ff20000000035008
  s8 : fffffffffffffff4 s9 : ffffffff86200610 s10: 0000000000000000
  s11: 0000000000000000 t3 : ffffffff8483dc30 t4 : ffffffff8483dc10
  t5 : ffffffff8483dbf0 t6 : ffffffff8483dbd0
 status: 0000000200000120 badaddr: ffffffff78204ffc cause: 000000000000000d
 [<ffffffff82958010>] bpf_struct_ops_test_run+0x28c/0x5fc
 [<ffffffff805083ee>] bpf_prog_test_run+0x170/0x548
 [<ffffffff805029c8>] __sys_bpf+0x2d2/0x378
 [<ffffffff804ff570>] __riscv_sys_bpf+0x5c/0x120
 [<ffffffff8000e8fe>] syscall_handler+0x62/0xe4
 [<ffffffff83362df6>] do_trap_ecall_u+0xc6/0x27c
 [<ffffffff833822c4>] ret_from_exception+0x0/0x64
 Code: b603 0109 b683 0189 b703 0209 8493 0609 157d 8d65 (a303) ffca
 ---[ end trace 0000000000000000 ]---
 Kernel panic - not syncing: Fatal exception
 SMP: stopping secondary CPUs

This patch improves the BPF JIT for the riscv64 architecture to emit kCFI
type id before BPF programs and struct ops trampolines.

After applying this patch, the above two selftests pass without any issues.

 root@rv-selftester:~/bpf# ./test_progs -a "rbtree_success,dummy_st_ops"
 #70/1    dummy_st_ops/dummy_st_ops_attach:OK
 #70/2    dummy_st_ops/dummy_init_ret_value:OK
 #70/3    dummy_st_ops/dummy_init_ptr_arg:OK
 #70/4    dummy_st_ops/dummy_multiple_args:OK
 #70/5    dummy_st_ops/dummy_sleepable:OK
 #70/6    dummy_st_ops/test_unsupported_field_sleepable:OK
 #70      dummy_st_ops:OK
 #189/1   rbtree_success/rbtree_add_nodes:OK
 #189/2   rbtree_success/rbtree_add_and_remove:OK
 #189/3   rbtree_success/rbtree_first_and_remove:OK
 #189/4   rbtree_success/rbtree_api_release_aliasing:OK
 #189     rbtree_success:OK
 Summary: 2/10 PASSED, 0 SKIPPED, 0 FAILED

 root@rv-selftester:~/bpf# zcat /proc/config.gz | grep CONFIG_CFI_CLANG
 CONFIG_CFI_CLANG=y

Puranjay Mohan (1):
  riscv64/cfi,bpf: Support kCFI + BPF on riscv64

 arch/riscv/include/asm/cfi.h    | 17 +++++++++++
 arch/riscv/kernel/cfi.c         | 53 +++++++++++++++++++++++++++++++++
 arch/riscv/net/bpf_jit.h        |  2 +-
 arch/riscv/net/bpf_jit_comp32.c |  2 +-
 arch/riscv/net/bpf_jit_comp64.c | 14 ++++++++-
 arch/riscv/net/bpf_jit_core.c   |  9 +++---
 6 files changed, 90 insertions(+), 7 deletions(-)

-- 
2.40.1


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH bpf-next 1/1] riscv64/cfi,bpf: Support kCFI + BPF on riscv64
  2024-03-03 17:02 [PATCH bpf-next 0/1] Support kCFI + BPF on riscv64 Puranjay Mohan
@ 2024-03-03 17:02 ` Puranjay Mohan
  2024-03-07 13:37   ` Pu Lehui
  2024-03-06 16:33 ` [PATCH bpf-next 0/1] " Björn Töpel
  2024-03-06 22:40 ` patchwork-bot+netdevbpf
  2 siblings, 1 reply; 5+ messages in thread
From: Puranjay Mohan @ 2024-03-03 17:02 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Luke Nelson,
	Xi Wang, Björn Töpel, Sami Tolvanen, Peter Zijlstra,
	Kees Cook, linux-riscv, linux-kernel, bpf
  Cc: puranjay12

The riscv BPF JIT doesn't emit proper kCFI prologues for BPF programs
and struct_ops trampolines when CONFIG_CFI_CLANG is enabled.

This causes CFI failures when calling BPF programs and can even crash
the kernel due to invalid memory accesses.

Example crash:

root@rv-selftester:~/bpf# ./test_progs -a dummy_st_ops

 Unable to handle kernel paging request at virtual address ffffffff78204ffc
 Oops [#1]
 Modules linked in: bpf_testmod(OE) [....]
 CPU: 3 PID: 356 Comm: test_progs Tainted: P           OE      6.8.0-rc1 #1
 Hardware name: riscv-virtio,qemu (DT)
 epc : bpf_struct_ops_test_run+0x28c/0x5fc
  ra : bpf_struct_ops_test_run+0x26c/0x5fc
 epc : ffffffff82958010 ra : ffffffff82957ff0 sp : ff200000007abc80
  gp : ffffffff868d6218 tp : ff6000008d87b840 t0 : 000000000000000f
  t1 : 0000000000000000 t2 : 000000002005793e s0 : ff200000007abcf0
  s1 : ff6000008a90fee0 a0 : 0000000000000000 a1 : 0000000000000000
  a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
  a5 : ffffffff868dba26 a6 : 0000000000000001 a7 : 0000000052464e43
  s2 : 00007ffffc0a95f0 s3 : ff6000008a90fe80 s4 : ff60000084c24c00
  s5 : ffffffff78205000 s6 : ff60000088750648 s7 : ff20000000035008
  s8 : fffffffffffffff4 s9 : ffffffff86200610 s10: 0000000000000000
  s11: 0000000000000000 t3 : ffffffff8483dc30 t4 : ffffffff8483dc10
  t5 : ffffffff8483dbf0 t6 : ffffffff8483dbd0
 status: 0000000200000120 badaddr: ffffffff78204ffc cause: 000000000000000d
 [<ffffffff82958010>] bpf_struct_ops_test_run+0x28c/0x5fc
 [<ffffffff805083ee>] bpf_prog_test_run+0x170/0x548
 [<ffffffff805029c8>] __sys_bpf+0x2d2/0x378
 [<ffffffff804ff570>] __riscv_sys_bpf+0x5c/0x120
 [<ffffffff8000e8fe>] syscall_handler+0x62/0xe4
 [<ffffffff83362df6>] do_trap_ecall_u+0xc6/0x27c
 [<ffffffff833822c4>] ret_from_exception+0x0/0x64
 Code: b603 0109 b683 0189 b703 0209 8493 0609 157d 8d65 (a303) ffca
 ---[ end trace 0000000000000000 ]---
 Kernel panic - not syncing: Fatal exception
 SMP: stopping secondary CPUs

Implement proper kCFI prologues for the BPF programs and callbacks and
drop __nocfi for riscv64. Fix the trampoline generation code to emit kCFI
prologue when a struct_ops trampoline is being prepared.

Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
---
 arch/riscv/include/asm/cfi.h    | 17 +++++++++++
 arch/riscv/kernel/cfi.c         | 53 +++++++++++++++++++++++++++++++++
 arch/riscv/net/bpf_jit.h        |  2 +-
 arch/riscv/net/bpf_jit_comp32.c |  2 +-
 arch/riscv/net/bpf_jit_comp64.c | 14 ++++++++-
 arch/riscv/net/bpf_jit_core.c   |  9 +++---
 6 files changed, 90 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/include/asm/cfi.h b/arch/riscv/include/asm/cfi.h
index 8f7a62257044..fb9696d7a3f2 100644
--- a/arch/riscv/include/asm/cfi.h
+++ b/arch/riscv/include/asm/cfi.h
@@ -13,11 +13,28 @@ struct pt_regs;
 
 #ifdef CONFIG_CFI_CLANG
 enum bug_trap_type handle_cfi_failure(struct pt_regs *regs);
+#define __bpfcall
+static inline int cfi_get_offset(void)
+{
+	return 4;
+}
+
+#define cfi_get_offset cfi_get_offset
+extern u32 cfi_bpf_hash;
+extern u32 cfi_bpf_subprog_hash;
+extern u32 cfi_get_func_hash(void *func);
 #else
 static inline enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
 {
 	return BUG_TRAP_TYPE_NONE;
 }
+
+#define cfi_bpf_hash 0U
+#define cfi_bpf_subprog_hash 0U
+static inline u32 cfi_get_func_hash(void *func)
+{
+	return 0;
+}
 #endif /* CONFIG_CFI_CLANG */
 
 #endif /* _ASM_RISCV_CFI_H */
diff --git a/arch/riscv/kernel/cfi.c b/arch/riscv/kernel/cfi.c
index 6ec9dbd7292e..64bdd3e1ab8c 100644
--- a/arch/riscv/kernel/cfi.c
+++ b/arch/riscv/kernel/cfi.c
@@ -75,3 +75,56 @@ enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
 
 	return report_cfi_failure(regs, regs->epc, &target, type);
 }
+
+#ifdef CONFIG_CFI_CLANG
+struct bpf_insn;
+
+/* Must match bpf_func_t / DEFINE_BPF_PROG_RUN() */
+extern unsigned int __bpf_prog_runX(const void *ctx,
+				    const struct bpf_insn *insn);
+
+/*
+ * Force a reference to the external symbol so the compiler generates
+ * __kcfi_typid.
+ */
+__ADDRESSABLE(__bpf_prog_runX);
+
+/* u32 __ro_after_init cfi_bpf_hash = __kcfi_typeid___bpf_prog_runX; */
+asm (
+"	.pushsection	.data..ro_after_init,\"aw\",@progbits	\n"
+"	.type	cfi_bpf_hash,@object				\n"
+"	.globl	cfi_bpf_hash					\n"
+"	.p2align	2, 0x0					\n"
+"cfi_bpf_hash:							\n"
+"	.word	__kcfi_typeid___bpf_prog_runX			\n"
+"	.size	cfi_bpf_hash, 4					\n"
+"	.popsection						\n"
+);
+
+/* Must match bpf_callback_t */
+extern u64 __bpf_callback_fn(u64, u64, u64, u64, u64);
+
+__ADDRESSABLE(__bpf_callback_fn);
+
+/* u32 __ro_after_init cfi_bpf_subprog_hash = __kcfi_typeid___bpf_callback_fn; */
+asm (
+"	.pushsection	.data..ro_after_init,\"aw\",@progbits	\n"
+"	.type	cfi_bpf_subprog_hash,@object			\n"
+"	.globl	cfi_bpf_subprog_hash				\n"
+"	.p2align	2, 0x0					\n"
+"cfi_bpf_subprog_hash:						\n"
+"	.word	__kcfi_typeid___bpf_callback_fn			\n"
+"	.size	cfi_bpf_subprog_hash, 4				\n"
+"	.popsection						\n"
+);
+
+u32 cfi_get_func_hash(void *func)
+{
+	u32 hash;
+
+	if (get_kernel_nofault(hash, func - cfi_get_offset()))
+		return 0;
+
+	return hash;
+}
+#endif
diff --git a/arch/riscv/net/bpf_jit.h b/arch/riscv/net/bpf_jit.h
index 8b35f12a4452..f4b6b3b9edda 100644
--- a/arch/riscv/net/bpf_jit.h
+++ b/arch/riscv/net/bpf_jit.h
@@ -1223,7 +1223,7 @@ static inline void emit_bswap(u8 rd, s32 imm, struct rv_jit_context *ctx)
 
 #endif /* __riscv_xlen == 64 */
 
-void bpf_jit_build_prologue(struct rv_jit_context *ctx);
+void bpf_jit_build_prologue(struct rv_jit_context *ctx, bool is_subprog);
 void bpf_jit_build_epilogue(struct rv_jit_context *ctx);
 
 int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
diff --git a/arch/riscv/net/bpf_jit_comp32.c b/arch/riscv/net/bpf_jit_comp32.c
index 529a83b85c1c..f5ba73bb153d 100644
--- a/arch/riscv/net/bpf_jit_comp32.c
+++ b/arch/riscv/net/bpf_jit_comp32.c
@@ -1301,7 +1301,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 	return 0;
 }
 
-void bpf_jit_build_prologue(struct rv_jit_context *ctx)
+void bpf_jit_build_prologue(struct rv_jit_context *ctx, bool is_subprog)
 {
 	const s8 *fp = bpf2rv32[BPF_REG_FP];
 	const s8 *r1 = bpf2rv32[BPF_REG_1];
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 869e4282a2c4..aac190085472 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -11,6 +11,7 @@
 #include <linux/memory.h>
 #include <linux/stop_machine.h>
 #include <asm/patch.h>
+#include <asm/cfi.h>
 #include "bpf_jit.h"
 
 #define RV_FENTRY_NINSNS 2
@@ -455,6 +456,12 @@ static int emit_call(u64 addr, bool fixed_addr, struct rv_jit_context *ctx)
 	return emit_jump_and_link(RV_REG_RA, off, fixed_addr, ctx);
 }
 
+static inline void emit_kcfi(u32 hash, struct rv_jit_context *ctx)
+{
+	if (IS_ENABLED(CONFIG_CFI_CLANG))
+		emit(hash, ctx);
+}
+
 static void emit_atomic(u8 rd, u8 rs, s16 off, s32 imm, bool is64,
 			struct rv_jit_context *ctx)
 {
@@ -869,6 +876,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 		emit_sd(RV_REG_SP, stack_size - 16, RV_REG_FP, ctx);
 		emit_addi(RV_REG_FP, RV_REG_SP, stack_size, ctx);
 	} else {
+		/* emit kcfi hash */
+		emit_kcfi(cfi_get_func_hash(func_addr), ctx);
 		/* For the trampoline called directly, just handle
 		 * the frame of trampoline.
 		 */
@@ -1711,7 +1720,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 	return 0;
 }
 
-void bpf_jit_build_prologue(struct rv_jit_context *ctx)
+void bpf_jit_build_prologue(struct rv_jit_context *ctx, bool is_subprog)
 {
 	int i, stack_adjust = 0, store_offset, bpf_stack_adjust;
 
@@ -1740,6 +1749,9 @@ void bpf_jit_build_prologue(struct rv_jit_context *ctx)
 
 	store_offset = stack_adjust - 8;
 
+	/* emit kcfi type preamble immediately before the  first insn */
+	emit_kcfi(is_subprog ? cfi_bpf_subprog_hash : cfi_bpf_hash, ctx);
+
 	/* nops reserved for auipc+jalr pair */
 	for (i = 0; i < RV_FENTRY_NINSNS; i++)
 		emit(rv_nop(), ctx);
diff --git a/arch/riscv/net/bpf_jit_core.c b/arch/riscv/net/bpf_jit_core.c
index 7b70ccb7fec3..6b3acac30c06 100644
--- a/arch/riscv/net/bpf_jit_core.c
+++ b/arch/riscv/net/bpf_jit_core.c
@@ -10,6 +10,7 @@
 #include <linux/filter.h>
 #include <linux/memory.h>
 #include <asm/patch.h>
+#include <asm/cfi.h>
 #include "bpf_jit.h"
 
 /* Number of iterations to try until offsets converge. */
@@ -100,7 +101,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 		pass++;
 		ctx->ninsns = 0;
 
-		bpf_jit_build_prologue(ctx);
+		bpf_jit_build_prologue(ctx, bpf_is_subprog(prog));
 		ctx->prologue_len = ctx->ninsns;
 
 		if (build_body(ctx, extra_pass, ctx->offset)) {
@@ -160,7 +161,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	ctx->ninsns = 0;
 	ctx->nexentries = 0;
 
-	bpf_jit_build_prologue(ctx);
+	bpf_jit_build_prologue(ctx, bpf_is_subprog(prog));
 	if (build_body(ctx, extra_pass, NULL)) {
 		prog = orig_prog;
 		goto out_free_hdr;
@@ -170,9 +171,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	if (bpf_jit_enable > 1)
 		bpf_jit_dump(prog->len, prog_size, pass, ctx->insns);
 
-	prog->bpf_func = (void *)ctx->ro_insns;
+	prog->bpf_func = (void *)ctx->ro_insns + cfi_get_offset();
 	prog->jited = 1;
-	prog->jited_len = prog_size;
+	prog->jited_len = prog_size - cfi_get_offset();
 
 	if (!prog->is_func || extra_pass) {
 		if (WARN_ON(bpf_jit_binary_pack_finalize(prog, jit_data->ro_header,
-- 
2.40.1


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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 0/1] Support kCFI + BPF on riscv64
  2024-03-03 17:02 [PATCH bpf-next 0/1] Support kCFI + BPF on riscv64 Puranjay Mohan
  2024-03-03 17:02 ` [PATCH bpf-next 1/1] riscv64/cfi,bpf: " Puranjay Mohan
@ 2024-03-06 16:33 ` Björn Töpel
  2024-03-06 22:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: Björn Töpel @ 2024-03-06 16:33 UTC (permalink / raw)
  To: Puranjay Mohan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Luke Nelson, Xi Wang, Sami Tolvanen, Peter Zijlstra, Kees Cook,
	linux-riscv, linux-kernel, bpf
  Cc: puranjay12

Puranjay Mohan <puranjay12@gmail.com> writes:

> With CONFIG_CFI_CLANG, the compiler injects a type preamble immediately
> before each function and a check to validate the target function type
> before indirect calls:
>
>   ; type preamble
>     .word <id>
>   function:
>     ...
>   ; indirect call check
>     lw      t1, -4(a0)
>     lui     t2, <hi20>
>     addiw   t2, t2, <lo12>
>     beq     t1, t2, .Ltmp0
>     ebreak
>   .Ltmp0:
>     jarl    a0
>
> BPF JIT currently doesn't emit this preamble before BPF programs and when
> the calling fuction tries to load the type id from the preamble, it finds
> an invalid value there.
>
> This will cause CFI failures like in the following bpf selftest:
>
> root@rv-selftester:~/bpf# ./test_progs -a "rbtree_success"
>
>  CFI failure at bpf_rbtree_add_impl+0x148/0x350 (target: bpf_prog_fb8b097ab47d164a_less+0x0/0x42; expected type: 0x00000000)
>  WARNING: CPU: 1 PID: 278 at bpf_rbtree_add_impl+0x148/0x350
>  Modules linked in: bpf_testmod(OE) drm fuse dm_mod backlight i2c_core configfs drm_panel_orientation_quirks ip_tables x_tables
>  CPU: 1 PID: 278 Comm: test_progs Tainted: P           OE      6.8.0-rc1 #1
>  Hardware name: riscv-virtio,qemu (DT)
>  epc : bpf_rbtree_add_impl+0x148/0x350
>   ra : bpf_prog_27b36e47d273751e_rbtree_first_and_remove+0x1aa/0x35e
>  epc : ffffffff805acc0c ra : ffffffff780077fa sp : ff2000000110b9d0
>   gp : ffffffff868d6218 tp : ff60000085772a40 t0 : ffffffff86849660
>   t1 : 0000000000000000 t2 : ffffffff9e4709a9 s0 : ff2000000110ba50
>   s1 : ff60000089c14958 a0 : ff60000089c14758 a1 : ff60000089c14958
>   a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
>   a5 : 0000000000000000 a6 : ff6000008aba4b30 a7 : ffffffff86849640
>   s2 : ff6000008aba4b30 s3 : ff60000089c14758 s4 : ffffffff780079f0
>   s5 : 0000000000000000 s6 : ffffffff84c01080 s7 : ff6000008aba4b30
>   s8 : 0000000000000000 s9 : 0000000000000000 s10: 0000000000000001
>   s11: 0000000000000000 t3 : ffffffff868499e0 t4 : ffffffff868499c0
>   t5 : ffffffff86849840 t6 : ffffffff86849860
>  status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
>  [<ffffffff805acc0c>] bpf_rbtree_add_impl+0x148/0x350
>  [<ffffffff780077fa>] bpf_prog_27b36e47d273751e_rbtree_first_and_remove+0x1aa/0x35e
>  [<ffffffff8294f32c>] bpf_test_run+0x2a4/0xa3c
>  [<ffffffff8294d032>] bpf_prog_test_run_skb+0x47a/0xe52
>  [<ffffffff805083ee>] bpf_prog_test_run+0x170/0x548
>  [<ffffffff805029c8>] __sys_bpf+0x2d2/0x378
>  [<ffffffff804ff570>] __riscv_sys_bpf+0x5c/0x120
>  [<ffffffff8000e8fe>] syscall_handler+0x62/0xe4
>  [<ffffffff83362df6>] do_trap_ecall_u+0xc6/0x27c
>  [<ffffffff833822c4>] ret_from_exception+0x0/0x64
>  ---[ end trace 0000000000000000 ]---
>
> The calling function tries to load the type id hash from target_func - 4.
> If this memory address is not mapped then it can cause a page fault and
> crash the kernel.
>
> This behaviour can be seen by running the 'dummy_st_ops' selftest:
>
> root@rv-selftester:~/bpf# ./test_progs -a dummy_st_ops
>
>  Unable to handle kernel paging request at virtual address ffffffff78204ffc
>  Oops [#1]
>  Modules linked in: bpf_testmod(OE) drm fuse backlight i2c_core drm_panel_orientation_quirks dm_mod configfs ip_tables x_tables [last unloaded: bpf_testmod(OE)]
>  CPU: 3 PID: 356 Comm: test_progs Tainted: P           OE      6.8.0-rc1 #1
>  Hardware name: riscv-virtio,qemu (DT)
>  epc : bpf_struct_ops_test_run+0x28c/0x5fc
>   ra : bpf_struct_ops_test_run+0x26c/0x5fc
>  epc : ffffffff82958010 ra : ffffffff82957ff0 sp : ff200000007abc80
>   gp : ffffffff868d6218 tp : ff6000008d87b840 t0 : 000000000000000f
>   t1 : 0000000000000000 t2 : 000000002005793e s0 : ff200000007abcf0
>   s1 : ff6000008a90fee0 a0 : 0000000000000000 a1 : 0000000000000000
>   a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000
>   a5 : ffffffff868dba26 a6 : 0000000000000001 a7 : 0000000052464e43
>   s2 : 00007ffffc0a95f0 s3 : ff6000008a90fe80 s4 : ff60000084c24c00
>   s5 : ffffffff78205000 s6 : ff60000088750648 s7 : ff20000000035008
>   s8 : fffffffffffffff4 s9 : ffffffff86200610 s10: 0000000000000000
>   s11: 0000000000000000 t3 : ffffffff8483dc30 t4 : ffffffff8483dc10
>   t5 : ffffffff8483dbf0 t6 : ffffffff8483dbd0
>  status: 0000000200000120 badaddr: ffffffff78204ffc cause: 000000000000000d
>  [<ffffffff82958010>] bpf_struct_ops_test_run+0x28c/0x5fc
>  [<ffffffff805083ee>] bpf_prog_test_run+0x170/0x548
>  [<ffffffff805029c8>] __sys_bpf+0x2d2/0x378
>  [<ffffffff804ff570>] __riscv_sys_bpf+0x5c/0x120
>  [<ffffffff8000e8fe>] syscall_handler+0x62/0xe4
>  [<ffffffff83362df6>] do_trap_ecall_u+0xc6/0x27c
>  [<ffffffff833822c4>] ret_from_exception+0x0/0x64
>  Code: b603 0109 b683 0189 b703 0209 8493 0609 157d 8d65 (a303) ffca
>  ---[ end trace 0000000000000000 ]---
>  Kernel panic - not syncing: Fatal exception
>  SMP: stopping secondary CPUs
>
> This patch improves the BPF JIT for the riscv64 architecture to emit kCFI
> type id before BPF programs and struct ops trampolines.
>
> After applying this patch, the above two selftests pass without any issues.
>
>  root@rv-selftester:~/bpf# ./test_progs -a "rbtree_success,dummy_st_ops"
>  #70/1    dummy_st_ops/dummy_st_ops_attach:OK
>  #70/2    dummy_st_ops/dummy_init_ret_value:OK
>  #70/3    dummy_st_ops/dummy_init_ptr_arg:OK
>  #70/4    dummy_st_ops/dummy_multiple_args:OK
>  #70/5    dummy_st_ops/dummy_sleepable:OK
>  #70/6    dummy_st_ops/test_unsupported_field_sleepable:OK
>  #70      dummy_st_ops:OK
>  #189/1   rbtree_success/rbtree_add_nodes:OK
>  #189/2   rbtree_success/rbtree_add_and_remove:OK
>  #189/3   rbtree_success/rbtree_first_and_remove:OK
>  #189/4   rbtree_success/rbtree_api_release_aliasing:OK
>  #189     rbtree_success:OK
>  Summary: 2/10 PASSED, 0 SKIPPED, 0 FAILED
>
>  root@rv-selftester:~/bpf# zcat /proc/config.gz | grep CONFIG_CFI_CLANG
>  CONFIG_CFI_CLANG=y

Apologies for the slow review. Nice work!

Acked-by: Björn Töpel <bjorn@kernel.org>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 0/1] Support kCFI + BPF on riscv64
  2024-03-03 17:02 [PATCH bpf-next 0/1] Support kCFI + BPF on riscv64 Puranjay Mohan
  2024-03-03 17:02 ` [PATCH bpf-next 1/1] riscv64/cfi,bpf: " Puranjay Mohan
  2024-03-06 16:33 ` [PATCH bpf-next 0/1] " Björn Töpel
@ 2024-03-06 22:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-03-06 22:40 UTC (permalink / raw)
  To: Puranjay Mohan
  Cc: paul.walmsley, palmer, aou, ast, daniel, andrii, martin.lau,
	eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa, luke.r.nels, xi.wang, bjorn, samitolvanen, peterz,
	keescook, linux-riscv, linux-kernel, bpf

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Sun,  3 Mar 2024 17:02:06 +0000 you wrote:
> With CONFIG_CFI_CLANG, the compiler injects a type preamble immediately
> before each function and a check to validate the target function type
> before indirect calls:
> 
>   ; type preamble
>     .word <id>
>   function:
>     ...
>   ; indirect call check
>     lw      t1, -4(a0)
>     lui     t2, <hi20>
>     addiw   t2, t2, <lo12>
>     beq     t1, t2, .Ltmp0
>     ebreak
>   .Ltmp0:
>     jarl    a0
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/1] riscv64/cfi,bpf: Support kCFI + BPF on riscv64
    https://git.kernel.org/bpf/bpf-next/c/dfd646d117b7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next 1/1] riscv64/cfi,bpf: Support kCFI + BPF on riscv64
  2024-03-03 17:02 ` [PATCH bpf-next 1/1] riscv64/cfi,bpf: " Puranjay Mohan
@ 2024-03-07 13:37   ` Pu Lehui
  0 siblings, 0 replies; 5+ messages in thread
From: Pu Lehui @ 2024-03-07 13:37 UTC (permalink / raw)
  To: Puranjay Mohan, bpf, linux-riscv, linux-kernel
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Luke Nelson,
	Xi Wang, Björn Töpel, Sami Tolvanen, Peter Zijlstra,
	Kees Cook


On 2024/3/4 1:02, Puranjay Mohan wrote:
> The riscv BPF JIT doesn't emit proper kCFI prologues for BPF programs

[SNIP]

>   
> -void bpf_jit_build_prologue(struct rv_jit_context *ctx)
> +void bpf_jit_build_prologue(struct rv_jit_context *ctx, bool is_subprog)

Not tracked in time. Some nits, although it has been merged. We don't 
need to add new parameters here since we can fetch prog in ctx. Others, 
it looks great.

>   {
>   	int i, stack_adjust = 0, store_offset, bpf_stack_adjust;
>   
> @@ -1740,6 +1749,9 @@ void bpf_jit_build_prologue(struct rv_jit_context *ctx)
>   
>   	store_offset = stack_adjust - 8;


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-03-07 13:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-03 17:02 [PATCH bpf-next 0/1] Support kCFI + BPF on riscv64 Puranjay Mohan
2024-03-03 17:02 ` [PATCH bpf-next 1/1] riscv64/cfi,bpf: " Puranjay Mohan
2024-03-07 13:37   ` Pu Lehui
2024-03-06 16:33 ` [PATCH bpf-next 0/1] " Björn Töpel
2024-03-06 22:40 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox