In an effort to actually test the refcounting logic at runtime, add a refcount_t member to prog_test_ref_kfunc and use it in selftests to verify and test the whole logic more exhaustively. To ensure reading the count to verify it remains stable, make prog_test_ref_kfunc a per-CPU variable, so that inside a BPF program the count can be read reliably based on number of acquisitions made. Then, pairing them with releases and reading from the global per-CPU variable will allow verifying whether release operations put the refcount. We don't actually rely on preemption being disabled, but migration must be disabled, as BPF program is the only context where these acquire and release functions are called. As such, when an object is acquired on one CPU, only that CPU should manipulate its refcount. Likewise, for bpf_this_cpu_ptr, the returned pointer and acquired pointer must match, so that refcount can actually be read and verified. The kfunc calls for prog_test_member do not require runtime refcounting, as they are only used for verifier selftests, not during runtime execution. Hence, their implementation now has a WARN_ON_ONCE as it is not meant to be reachable code at runtime. It is strictly used in tests triggering failure cases in the verifier. bpf_kfunc_call_memb_release is called from map free path, since prog_test_member is embedded in map value for some verifier tests, so we skip WARN_ON_ONCE for it. Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> --- net/bpf/test_run.c | 32 +++++++++++++------ .../testing/selftests/bpf/verifier/map_kptr.c | 4 +-- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 7a1579c91432..16d489b03000 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -564,31 +564,39 @@ struct prog_test_ref_kfunc { int b; struct prog_test_member memb; struct prog_test_ref_kfunc *next; + refcount_t cnt; }; -static struct prog_test_ref_kfunc prog_test_struct = { +DEFINE_PER_CPU(struct prog_test_ref_kfunc, prog_test_struct) = { .a = 42, .b = 108, - .next = &prog_test_struct, + .cnt = REFCOUNT_INIT(1), }; noinline struct prog_test_ref_kfunc * bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr) { - /* randomly return NULL */ - if (get_jiffies_64() % 2) - return NULL; - return &prog_test_struct; + struct prog_test_ref_kfunc *p; + + p = this_cpu_ptr(&prog_test_struct); + p->next = p; + refcount_inc(&p->cnt); + return p; } noinline struct prog_test_member * bpf_kfunc_call_memb_acquire(void) { - return &prog_test_struct.memb; + WARN_ON_ONCE(1); + return NULL; } noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) { + if (!p) + return; + + refcount_dec(&p->cnt); } noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p) @@ -597,12 +605,18 @@ noinline void bpf_kfunc_call_memb_release(struct prog_test_member *p) noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p) { + WARN_ON_ONCE(1); } noinline struct prog_test_ref_kfunc * -bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b) +bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **pp, int a, int b) { - return &prog_test_struct; + struct prog_test_ref_kfunc *p = READ_ONCE(*pp); + + if (!p) + return NULL; + refcount_inc(&p->cnt); + return p; } struct prog_test_pass1 { diff --git a/tools/testing/selftests/bpf/verifier/map_kptr.c b/tools/testing/selftests/bpf/verifier/map_kptr.c index 9113834640e6..6914904344c0 100644 --- a/tools/testing/selftests/bpf/verifier/map_kptr.c +++ b/tools/testing/selftests/bpf/verifier/map_kptr.c @@ -212,13 +212,13 @@ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 0), BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1), BPF_EXIT_INSN(), - BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 24), + BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 32), BPF_EXIT_INSN(), }, .prog_type = BPF_PROG_TYPE_SCHED_CLS, .fixup_map_kptr = { 1 }, .result = REJECT, - .errstr = "access beyond struct prog_test_ref_kfunc at off 24 size 8", + .errstr = "access beyond struct prog_test_ref_kfunc at off 32 size 8", }, { "map_kptr: unref: inherit PTR_UNTRUSTED on struct walk", -- 2.35.1