Puranjay Mohan <puranjay12@xxxxxxxxx> 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@xxxxxxxxxx>
