| Message ID | 20240303170207.82201-1-puranjay12@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Support kCFI + BPF on riscv64 | expand |
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>
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!
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(-)