On Fri, Dec 8, 2023 at 2:46 PM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > > Ok, did that. Current patches (on top of bpf-next) are here: > > git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git x86/cfi Looks really great. The last patch is cleaner than I expected. Good idea. > (really should try and write better changelogs, but it's too late) commit logs look fine except the "pilfer" word that I had to look up in the dictionary :) > [ 247.721063] ? bpf_throw+0x9b/0xf0 > [ 247.721126] ? bpf_test_run+0x108/0x350 > [ 247.721191] ? bpf_prog_5555714b685bf0cf_exception_throw_always_1+0x26/0x26 > [ 247.721301] ? bpf_test_run+0x108/0x350 > [ 247.721368] bpf_test_run+0x212/0x350 > [ 247.721433] ? slab_build_skb+0x22/0x110 > [ 247.721503] bpf_prog_test_run_skb+0x347/0x4a0 > > But I'm too tired to think staight. Is this a bpf_callback_t vs > bpf_exception_cb difference? Yep. It's easy to fix: diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 0e162eae8639..e36b3f41751e 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1484,7 +1484,7 @@ struct bpf_prog_aux { int cgroup_atype; /* enum cgroup_bpf_attach_type */ struct bpf_map *cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]; char name[BPF_OBJ_NAME_LEN]; - unsigned int (*bpf_exception_cb)(u64 cookie, u64 sp, u64 bp); + u64 (*bpf_exception_cb)(u64 cookie, u64 sp, u64 bp, u64, u64); #ifdef CONFIG_SECURITY void *security; #endif diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index fe229b28e4a9..650ebe8ff183 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -2537,7 +2537,7 @@ __bpf_kfunc void bpf_throw(u64 cookie) * which skips compiler generated instrumentation to do the same. */ kasan_unpoison_task_stack_below((void *)(long)ctx.sp); - ctx.aux->bpf_exception_cb(cookie, ctx.sp, ctx.bp); + ctx.aux->bpf_exception_cb(cookie, ctx.sp, ctx.bp, 0, 0); WARN(1, "A call to BPF exception callback should never return\n"); } and with that all of test_progs runs successfully without CFI panics. *happy dance* Only test_progs -t btf/line_info fails suspiciously. There we check that line info embedded in the prog looks sane. New cfi preamble is probably tripping something. It could be a test issue. I'll investigate. It's not a blocker. Do you mind resending the whole set so that BPF CI can test it on different archs ?