* [PATCH bpf-next v1 0/2] Fix global subprog verification context
@ 2026-09-05 3:40 Kumar Kartikeya Dwivedi
2026-09-05 3:40 ` [PATCH bpf-next v1 1/2] bpf: Verify global subprogs in each sleepability context Kumar Kartikeya Dwivedi
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-09-05 3:40 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Eduard Zingerman, Emil Tsalapatis, Nicholas Carlini, kkd,
kernel-team
Global subprogs can be invoked in both sleepable and non-sleepable
contexts, but are verified based on context assumptions for the program
type as a whole and not the precise context in which they are invoked.
Nicholas identified that this can cause verifier's safety assumptions to
be violated. Implement a fix by performing verification twice but only
if different contexts are exercised, and add selftests to ensure coverage.
Kumar Kartikeya Dwivedi (2):
bpf: Verify global subprogs in each sleepability context
selftests/bpf: Test global subprog callback contexts
include/linux/bpf.h | 5 +-
kernel/bpf/verifier.c | 76 ++++++----
.../bpf/progs/verifier_async_cb_context.c | 131 ++++++++++++++++++
3 files changed, 180 insertions(+), 32 deletions(-)
base-commit: 3ccdb07813829ba9487273e75d1cb238cfa774c1
--
2.53.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH bpf-next v1 1/2] bpf: Verify global subprogs in each sleepability context 2026-09-05 3:40 [PATCH bpf-next v1 0/2] Fix global subprog verification context Kumar Kartikeya Dwivedi @ 2026-09-05 3:40 ` Kumar Kartikeya Dwivedi 2026-09-05 3:54 ` sashiko-bot 2026-09-05 4:39 ` bot+bpf-ci 2026-09-05 3:40 ` [PATCH bpf-next v1 2/2] selftests/bpf: Test global subprog callback contexts Kumar Kartikeya Dwivedi 2026-09-05 4:33 ` [PATCH bpf-next v1 0/2] Fix global subprog verification context Kumar Kartikeya Dwivedi 2 siblings, 2 replies; 7+ messages in thread From: Kumar Kartikeya Dwivedi @ 2026-09-05 3:40 UTC (permalink / raw) To: bpf Cc: Nicholas Carlini, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Eduard Zingerman, Emil Tsalapatis, kkd, kernel-team Global subprograms are verified independently with a fresh verifier root. do_check_common() currently seeds that root's in_sleepable state from the program, even though a global subprogram can also run from callbacks whose execution context differs from the program's main entry point. In particular, workqueue and task-work callbacks are sleepable even when the containing program is not. A global subprogram of that program is therefore verified as non-sleepable, making in_rcu_cs() true and allowing loads of RCU-protected kptrs to produce trusted MEM_RCU pointers. The same subprogram can then be called from a sleepable callback without a classic RCU reader. It can retain such a pointer while the object is freed and use it after free. The verifier's execution-context predicates are complementary. A state is sleepable only when in_sleepable is set and no RCU, preemption, IRQ, or lock region is active. Each condition which prevents sleeping also provides RCU protection, while in_rcu_cs() treats a non-sleepable state as implicitly protected. Use this relationship to represent a global subprogram caller with only the result of in_sleepable_context(). A protected sleepable caller is normalized to in_sleepable=false at the independent verification root. This both prevents sleepable operations and makes in_rcu_cs() true without copying caller-owned lock state. Record whether each global subprogram is called with either in_sleepable value and verify it once for every observed value. Walk global subprograms in caller-before-callee order so the values propagate through global call chains, and repeat until every discovered context has been verified to cover asynchronous callback cycles. This makes an unprotected callback verify the global subprogram as sleepable, turning its RCU-protected kptr load into an untrusted pointer. Protected callers and global subprograms which do not depend on implicit RCU protection remain valid. Fixes: 81f1d7a583fa ("bpf: wq: add bpf_wq_set_callback_impl") Fixes: 38aa7003e369 ("bpf: task work scheduling kfuncs") Reported-by: Nicholas Carlini <npc@anthropic.com> Suggested-by: Nicholas Carlini <npc@anthropic.com> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> --- include/linux/bpf.h | 5 +-- kernel/bpf/verifier.c | 76 ++++++++++++++++++++++++++----------------- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 3a7eb2185c35..66d04244c737 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1650,8 +1650,9 @@ static inline void bpf_trampoline_set_flags(struct bpf_trampoline *tr, u32 flags struct bpf_func_info_aux { u16 linkage; bool unreliable; - bool called : 1; - bool verified : 1; + /* Indexed by in_sleepable. */ + bool called[2]; + bool verified[2]; }; enum bpf_jit_poke_reason { diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 5b51e7ee1a3f..4b9aa0f168bb 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9957,6 +9957,7 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, if (err == -EFAULT) return err; if (bpf_subprog_is_global(env, subprog)) { + struct bpf_func_info_aux *sub_aux = subprog_aux(env, subprog); const char *sub_name = bpf_subprog_name(env, subprog); const char *operation; bool returns_void; @@ -9988,11 +9989,10 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, if (env->log.level & BPF_LOG_LEVEL) verbose(env, "Func#%d ('%s') is global and assumed valid.\n", subprog, sub_name); + sub_aux->called[in_sleepable_context(env)] = true; returns_void = subprog_returns_void(env, subprog); if (env->subprog_info[subprog].changes_pkt_data) clear_all_pkt_pointers(env); - /* mark global subprog for verifying after main prog */ - subprog_aux(env, subprog)->called = true; if (returns_void) bpf_diag_record_scrub(env, &caller->regs[BPF_REG_0], BPF_DIAG_MOD_CALLER_SAVED); else @@ -10804,7 +10804,12 @@ int bpf_get_helper_proto(struct bpf_verifier_env *env, int func_id, return *ptr && (*ptr)->func ? 0 : -EINVAL; } -/* Check if we're in a sleepable context. */ +/* + * This predicate is the inverse of in_rcu_cs(): non-sleepable programs and + * every condition that prevents sleeping also provide RCU protection. Global + * subprog verification relies on this equivalence to represent the caller's + * execution context using only the in_sleepable bit. + */ static inline bool in_sleepable_context(struct bpf_verifier_env *env) { return !env->cur_state->active_rcu_locks && @@ -19560,13 +19565,14 @@ static void free_states(struct bpf_verifier_env *env) } } -static int do_check_common(struct bpf_verifier_env *env, int subprog) +static int do_check_common(struct bpf_verifier_env *env, int subprog, bool in_sleepable) { bool pop_log = !(env->log.level & BPF_LOG_LEVEL2); struct bpf_subprog_info *sub = subprog_info(env, subprog); struct bpf_prog_aux *aux = env->prog->aux; struct bpf_verifier_state *state; struct bpf_reg_state *regs; + u32 old_insns_total = sub->insns_total; u32 insn_processed = env->insn_processed; int ret, i; @@ -19579,7 +19585,7 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog) state->curframe = 0; state->speculative = false; state->branches = 1; - state->in_sleepable = env->prog->sleepable; + state->in_sleepable = in_sleepable; state->frame[0] = kzalloc_obj(struct bpf_func_state, GFP_KERNEL_ACCOUNT); if (!state->frame[0]) { kfree(state); @@ -19721,7 +19727,8 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog) * Accumulate their total counts as total counts of the main or * global subprog hosting the async call. */ - env->subprog_info[subprog].insns_total = env->insn_processed - insn_processed; + env->subprog_info[subprog].insns_total = old_insns_total + + (env->insn_processed - insn_processed); return ret; } @@ -19749,45 +19756,54 @@ static int do_check_subprogs(struct bpf_verifier_env *env) { struct bpf_prog_aux *aux = env->prog->aux; struct bpf_func_info_aux *sub_aux; - int i, ret, new_cnt; + int context, i, j, ret, new_cnt; if (!aux->func_info) return 0; /* exception callback is presumed to be always called */ - if (env->exception_callback_subprog) - subprog_aux(env, env->exception_callback_subprog)->called = true; + if (env->exception_callback_subprog) { + sub_aux = subprog_aux(env, env->exception_callback_subprog); + sub_aux->called[env->prog->sleepable] = true; + } again: new_cnt = 0; - for (i = 1; i < env->subprog_cnt; i++) { + /* + * Walk callers before callees so each global subprog normally sees all + * of its contexts before it is verified. Async callback cycles can add a + * context to an earlier subprog, so repeat until every called context is + * verified. + */ + for (j = env->subprog_cnt - 1; j >= 0; j--) { + i = env->subprog_topo_order[j]; + if (!i) + continue; if (!bpf_subprog_is_global(env, i)) continue; sub_aux = subprog_aux(env, i); - if (!sub_aux->called || sub_aux->verified) - continue; + for (context = 0; context < ARRAY_SIZE(sub_aux->called); context++) { + if (!sub_aux->called[context] || sub_aux->verified[context]) + continue; - env->insn_idx = env->subprog_info[i].start; - WARN_ON_ONCE(env->insn_idx == 0); - ret = do_check_common(env, i); - if (ret) { - return ret; - } else if (env->log.level & BPF_LOG_LEVEL) { - verbose(env, "Func#%d ('%s') is safe for any args that match its prototype\n", - i, bpf_subprog_name(env, i)); - } + env->insn_idx = env->subprog_info[i].start; + WARN_ON_ONCE(env->insn_idx == 0); + ret = do_check_common(env, i, context); + if (ret) + return ret; + if (env->log.level & BPF_LOG_LEVEL) + verbose(env, "Func#%d ('%s') is safe for any args " + "that match its prototype\n", + i, bpf_subprog_name(env, i)); - /* We verified new global subprog, it might have called some - * more global subprogs that we haven't verified yet, so we - * need to do another pass over subprogs to verify those. - */ - sub_aux->verified = true; - new_cnt++; + sub_aux->verified[context] = true; + new_cnt++; + } } - /* We can't loop forever as we verify at least one global subprog on - * each pass. + /* We can't loop forever as each pass verifies at least one new context, + * and there are only two contexts per global subprog. */ if (new_cnt) goto again; @@ -19800,7 +19816,7 @@ static int do_check_main(struct bpf_verifier_env *env) int ret; env->insn_idx = 0; - ret = do_check_common(env, 0); + ret = do_check_common(env, 0, env->prog->sleepable); if (!ret) env->prog->aux->stack_depth = env->subprog_info[0].stack_depth; return ret; -- 2.53.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v1 1/2] bpf: Verify global subprogs in each sleepability context 2026-09-05 3:40 ` [PATCH bpf-next v1 1/2] bpf: Verify global subprogs in each sleepability context Kumar Kartikeya Dwivedi @ 2026-09-05 3:54 ` sashiko-bot 2026-09-05 4:39 ` bot+bpf-ci 1 sibling, 0 replies; 7+ messages in thread From: sashiko-bot @ 2026-09-05 3:54 UTC (permalink / raw) To: Kumar Kartikeya Dwivedi; +Cc: bpf Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] BPF subsystem multi-line comment style violation. -- commit aea430d1f0450a56e5505b581236a015c8f1396b Author: Kumar Kartikeya Dwivedi <memxor@gmail.com> bpf: Verify global subprogs in each sleepability context This commit updates the BPF verifier to independently verify global subprograms across different sleepability contexts, rather than relying on the sleepable state of the main program. It modifies do_check_subprogs to track and iteratively verify contexts as they propagate through asynchronous callbacks. > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 5b51e7ee1a3f9..4b9aa0f168bb5 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c [ ... ] > @@ -19749,45 +19756,54 @@ static int do_check_subprogs(struct bpf_verifier_env *env) [ ... ] > - /* We can't loop forever as we verify at least one global subprog on > - * each pass. > + /* We can't loop forever as each pass verifies at least one new context, > + * and there are only two contexts per global subprog. > */ > if (new_cnt) > goto again; [Severity: Low] Does this new comment block violate the preferred multi-line comment style for the BPF subsystem? The official style mandates that multi-line comments should place the opening /* on its own line before the text begins. Could it be reformatted to look like this instead? /* * We can't loop forever as each pass verifies at least one new context, * and there are only two contexts per global subprog. */ -- Sashiko AI review · https://sashiko.dev/#/patchset/20260905034018.2095649-1-memxor@gmail.com?part=1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v1 1/2] bpf: Verify global subprogs in each sleepability context 2026-09-05 3:40 ` [PATCH bpf-next v1 1/2] bpf: Verify global subprogs in each sleepability context Kumar Kartikeya Dwivedi 2026-09-05 3:54 ` sashiko-bot @ 2026-09-05 4:39 ` bot+bpf-ci 1 sibling, 0 replies; 7+ messages in thread From: bot+bpf-ci @ 2026-09-05 4:39 UTC (permalink / raw) To: memxor, bpf Cc: npc, ast, andrii, daniel, eddyz87, emil, kkd, kernel-team, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, mason, ihor.solodrai [-- Attachment #1: Type: text/plain, Size: 2278 bytes --] > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 3a7eb2185c35..66d04244c737 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1650,8 +1650,9 @@ static inline void bpf_trampoline_set_flags(struct bpf_trampoline *tr, u32 flags > struct bpf_func_info_aux { > u16 linkage; > bool unreliable; > - bool called : 1; > - bool verified : 1; > + /* Indexed by in_sleepable. */ > + bool called[2]; > + bool verified[2]; > }; [ ... ] > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 5b51e7ee1a3f..4b9aa0f168bb 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c [ ... ] > @@ -19560,13 +19565,14 @@ static void free_states(struct bpf_verifier_env *env) > } > } > > -static int do_check_common(struct bpf_verifier_env *env, int subprog) > +static int do_check_common(struct bpf_verifier_env *env, int subprog, bool in_sleepable) > { This isn't a bug, but would it be worth naming this parameter is_sleepable instead? There's a file-scope helper in_sleepable() at kernel/bpf/verifier.c and the parameter now shadows it within this function, so any future code added here cannot call the helper without renaming. The sibling function that seeds the same field for async callbacks, push_async_cb(), uses the name is_sleepable for its equivalent parameter. > bool pop_log = !(env->log.level & BPF_LOG_LEVEL2); > struct bpf_subprog_info *sub = subprog_info(env, subprog); > struct bpf_prog_aux *aux = env->prog->aux; > struct bpf_verifier_state *state; > struct bpf_reg_state *regs; > + u32 old_insns_total = sub->insns_total; > u32 insn_processed = env->insn_processed; This isn't a bug, but could the commit message add a sentence about the insns_total accounting change here? A global subprog verified in both contexts now reports the sum of both passes in subprog_info[].insns_total, which affects BPF_LOG_STATS output and veristat per-subprog instruction counts. The code is correct but the behavioral change isn't mentioned in the changelog. [ ... ] --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33943332260 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next v1 2/2] selftests/bpf: Test global subprog callback contexts 2026-09-05 3:40 [PATCH bpf-next v1 0/2] Fix global subprog verification context Kumar Kartikeya Dwivedi 2026-09-05 3:40 ` [PATCH bpf-next v1 1/2] bpf: Verify global subprogs in each sleepability context Kumar Kartikeya Dwivedi @ 2026-09-05 3:40 ` Kumar Kartikeya Dwivedi 2026-09-05 4:39 ` bot+bpf-ci 2026-09-05 4:33 ` [PATCH bpf-next v1 0/2] Fix global subprog verification context Kumar Kartikeya Dwivedi 2 siblings, 1 reply; 7+ messages in thread From: Kumar Kartikeya Dwivedi @ 2026-09-05 3:40 UTC (permalink / raw) To: bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Eduard Zingerman, Emil Tsalapatis, Nicholas Carlini, kkd, kernel-team Exercise global subprogram verification from workqueue and task-work callbacks. Both callback types can run in a sleepable context even when the containing program is not sleepable, so an unprotected callback must not let the global subprogram use implicit RCU protection inherited from the program. Add negative cases which load an RCU-protected task kptr in a global subprogram reached from each callback type. The tests fail on an unfixed kernel because the programs are incorrectly accepted. Also cover a workqueue callback protected by an explicit RCU read-side critical section. Finally, call the same harmless global subprogram directly from the main program and from an unprotected callback. This requires both non-sleepable and sleepable verification roots and proves that global calls from callbacks are not rejected wholesale. Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> --- .../bpf/progs/verifier_async_cb_context.c | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c b/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c index 6bf95550a024..9ce7359913f8 100644 --- a/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c +++ b/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c @@ -9,6 +9,11 @@ char _license[] SEC("license") = "GPL"; +struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym; +void bpf_task_release(struct task_struct *p) __ksym; +void bpf_rcu_read_lock(void) __ksym; +void bpf_rcu_read_unlock(void) __ksym; + /* Timer tests */ struct timer_elem { @@ -66,6 +71,7 @@ int timer_sleepable_prog(void *ctx) struct wq_elem { struct bpf_wq w; + struct task_struct __kptr *task; }; struct { @@ -119,6 +125,105 @@ int wq_sleepable_prog(void *ctx) return 0; } +__noinline int wq_global_acquire(void) +{ + struct task_struct *task, *acquired; + struct wq_elem *val; + int key = 0; + + val = bpf_map_lookup_elem(&wq_map, &key); + if (!val) + return 0; + + task = val->task; + if (!task) + return 0; + + acquired = bpf_task_acquire(task); + if (acquired) + bpf_task_release(acquired); + return 0; +} + +static int wq_global_rcu_cb(void *map, int *key, void *value) +{ + return wq_global_acquire(); +} + +SEC("fentry/bpf_fentry_test1") +__failure __msg("R1 must be a rcu pointer") +int wq_global_rcu_prog(void *ctx) +{ + struct wq_elem *val; + int key = 0; + + val = bpf_map_lookup_elem(&wq_map, &key); + if (!val) + return 0; + + bpf_wq_init(&val->w, &wq_map, 0); + bpf_wq_set_callback(&val->w, wq_global_rcu_cb, 0); + return 0; +} + +static int wq_global_rcu_lock_cb(void *map, int *key, void *value) +{ + int ret; + + bpf_rcu_read_lock(); + ret = wq_global_acquire(); + bpf_rcu_read_unlock(); + return ret; +} + +SEC("fentry/bpf_fentry_test1") +__success +int wq_global_rcu_lock_prog(void *ctx) +{ + struct wq_elem *val; + int key = 0; + + /* Verify the same global subprog in non-sleepable and protected contexts. */ + wq_global_acquire(); + + val = bpf_map_lookup_elem(&wq_map, &key); + if (!val) + return 0; + + bpf_wq_init(&val->w, &wq_map, 0); + bpf_wq_set_callback(&val->w, wq_global_rcu_lock_cb, 0); + return 0; +} + +__noinline int wq_global_no_rcu(void) +{ + return 0; +} + +static int wq_global_no_rcu_cb(void *map, int *key, void *value) +{ + return wq_global_no_rcu(); +} + +SEC("fentry/bpf_fentry_test1") +__success +int wq_global_no_rcu_prog(void *ctx) +{ + struct wq_elem *val; + int key = 0; + + /* Verify the same global in non-sleepable and unprotected contexts. */ + wq_global_no_rcu(); + + val = bpf_map_lookup_elem(&wq_map, &key); + if (!val) + return 0; + + bpf_wq_init(&val->w, &wq_map, 0); + bpf_wq_set_callback(&val->w, wq_global_no_rcu_cb, 0); + return 0; +} + /* Task work tests */ struct task_work_elem { @@ -179,3 +284,29 @@ int task_work_sleepable_prog(void *ctx) bpf_task_work_schedule_resume(task, &val->tw, &task_work_map, task_work_cb); return 0; } + +static int task_work_global_rcu_cb(struct bpf_map *map, void *key, void *value) +{ + return wq_global_acquire(); +} + +SEC("fentry/bpf_fentry_test1") +__failure __msg("R1 must be a rcu pointer") +int task_work_global_rcu_prog(void *ctx) +{ + struct task_work_elem *val; + struct task_struct *task; + int key = 0; + + val = bpf_map_lookup_elem(&task_work_map, &key); + if (!val) + return 0; + + task = bpf_get_current_task_btf(); + if (!task) + return 0; + + bpf_task_work_schedule_resume(task, &val->tw, &task_work_map, + task_work_global_rcu_cb); + return 0; +} -- 2.53.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v1 2/2] selftests/bpf: Test global subprog callback contexts 2026-09-05 3:40 ` [PATCH bpf-next v1 2/2] selftests/bpf: Test global subprog callback contexts Kumar Kartikeya Dwivedi @ 2026-09-05 4:39 ` bot+bpf-ci 0 siblings, 0 replies; 7+ messages in thread From: bot+bpf-ci @ 2026-09-05 4:39 UTC (permalink / raw) To: memxor, bpf Cc: ast, andrii, daniel, eddyz87, emil, npc, kkd, kernel-team, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, mason, ihor.solodrai [-- Attachment #1: Type: text/plain, Size: 8770 bytes --] > diff --git a/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c b/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c > index 6bf95550a024..9ce7359913f8 100644 > --- a/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c > +++ b/tools/testing/selftests/bpf/progs/verifier_async_cb_context.c > @@ -9,6 +9,11 @@ > > char _license[] SEC("license") = "GPL"; > > +struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym; > +void bpf_task_release(struct task_struct *p) __ksym; > +void bpf_rcu_read_lock(void) __ksym; > +void bpf_rcu_read_unlock(void) __ksym; > + > /* Timer tests */ > > struct timer_elem { [ ... ] > @@ -66,6 +71,7 @@ int timer_sleepable_prog(void *ctx) > > struct wq_elem { > struct bpf_wq w; > + struct task_struct __kptr *task; > }; > > struct { [ ... ] > @@ -119,6 +125,105 @@ int wq_sleepable_prog(void *ctx) > return 0; > } > > +__noinline int wq_global_acquire(void) > +{ > + struct task_struct *task, *acquired; > + struct wq_elem *val; > + int key = 0; > + > + val = bpf_map_lookup_elem(&wq_map, &key); > + if (!val) > + return 0; > + > + task = val->task; > + if (!task) > + return 0; > + > + acquired = bpf_task_acquire(task); > + if (acquired) > + bpf_task_release(acquired); > + return 0; > +} > + > +static int wq_global_rcu_cb(void *map, int *key, void *value) > +{ > + return wq_global_acquire(); > +} > + > +SEC("fentry/bpf_fentry_test1") > +__failure __msg("R1 must be a rcu pointer") > +int wq_global_rcu_prog(void *ctx) > +{ > + struct wq_elem *val; > + int key = 0; > + > + val = bpf_map_lookup_elem(&wq_map, &key); > + if (!val) > + return 0; > + > + bpf_wq_init(&val->w, &wq_map, 0); > + bpf_wq_set_callback(&val->w, wq_global_rcu_cb, 0); > + return 0; > +} > + > +static int wq_global_rcu_lock_cb(void *map, int *key, void *value) > +{ > + int ret; > + > + bpf_rcu_read_lock(); > + ret = wq_global_acquire(); > + bpf_rcu_read_unlock(); > + return ret; > +} Does wq_global_rcu_lock_cb() verify with this return value? Workqueue callbacks must return exactly 0. When bpf_wq_set_callback is processed, the callback's return range gets constrained to [0, 0] in set_timer_callback_state() via: kernel/bpf/verifier.c:set_timer_callback_state() { ... callee->callback_ret_range = retval_range(0, 0); callee->in_async_callback_fn = true; ... } Later when the callback returns, check_return_code() enforces this: kernel/bpf/verifier.c:check_return_code() { ... if (frame->in_async_callback_fn) { range = frame->callback_ret_range; goto enforce_retval; } ... } A call to a global subprogram leaves R0 as an unbounded scalar. In check_func_call(), the bpf_subprog_is_global() path does: kernel/bpf/verifier.c:check_func_call() { ... if (bpf_subprog_is_global(env, subprog)) { ... mark_reg_unknown(env, caller->regs, ret_regs[0]); ... } ... } So returning ret here only verifies because the compiler constant-folds wq_global_acquire()'s return to 0 (every path returns 0). The same issue appears in wq_global_rcu_cb() above, which also returns the result of wq_global_acquire(). If wq_global_acquire() ever gets a non-zero return path, or if the optimizer doesn't fold it, wq_global_rcu_lock_prog's __success annotation fails with "At async callback return the register R0 has ... should have been in [0, 0]", and wq_global_rcu_prog's __msg("R1 must be a rcu pointer") never appears because bpf_check() runs do_check_main() before do_check_subprogs() and the callback's return-code error aborts verification early. The existing callbacks in progs/wq.c all return literal constants (wq.c:wq_cb_sleepable() and wq.c:wq_cb()). Would `wq_global_acquire(); return 0;` in both wq_global_rcu_cb and wq_global_rcu_lock_cb remove the dependency on optimizer behavior? > + > +SEC("fentry/bpf_fentry_test1") > +__success > +int wq_global_rcu_lock_prog(void *ctx) > +{ > + struct wq_elem *val; > + int key = 0; > + > + /* Verify the same global subprog in non-sleepable and protected contexts. */ > + wq_global_acquire(); > + > + val = bpf_map_lookup_elem(&wq_map, &key); > + if (!val) > + return 0; > + > + bpf_wq_init(&val->w, &wq_map, 0); > + bpf_wq_set_callback(&val->w, wq_global_rcu_lock_cb, 0); > + return 0; > +} > + > +__noinline int wq_global_no_rcu(void) > +{ > + return 0; > +} > + > +static int wq_global_no_rcu_cb(void *map, int *key, void *value) > +{ > + return wq_global_no_rcu(); > +} > + > +SEC("fentry/bpf_fentry_test1") > +__success > +int wq_global_no_rcu_prog(void *ctx) > +{ > + struct wq_elem *val; > + int key = 0; > + > + /* Verify the same global in non-sleepable and unprotected contexts. */ > + wq_global_no_rcu(); Can the calls to wq_global_no_rcu() get optimized away? The function has an empty body (return 0), takes no arguments, is not __weak, and its address is never taken. At -O2 (which selftests always use via tools/testing/selftests/bpf/Makefile), LLVM can infer memory(none)/willreturn/nounwind and constant-propagate the return value. Both call sites then become dead: the direct call here (result discarded) and the call in wq_global_no_rcu_cb() (whose result IPSCCP replaces with 0). The __noinline attribute only blocks inlining, not interprocedural attribute inference or DCE. If the calls are elided, there is no relocation to the subprog, libbpf never appends it to the program, check_func_call() never runs the bpf_subprog_is_global() path: kernel/bpf/verifier.c:check_func_call() { ... if (bpf_subprog_is_global(env, subprog)) { ... } ... } and do_check_subprogs() never calls do_check_common(env, subprog, 0) or do_check_common(env, subprog, 1) for it: kernel/bpf/verifier.c:do_check_subprogs() { for (i = 1; i < env->subprog_cnt; i++) { ... if (sub_aux->called[0]) ret = do_check_common(env, i, 0); if (sub_aux->called[1]) ret = do_check_common(env, i, 1); ... } } The test still passes __success because wq_global_no_rcu_prog loads successfully, but it covers nothing. This is the only test that exercises successful sleepable-root verification of a global subprog and the caller-before-callee/repeat loop that patch 1/2 adds. The changelog claims "call the same harmless global subprogram directly from the main program and from an unprotected callback. This requires both non-sleepable and sleepable verification roots and proves that global calls from callbacks are not rejected wholesale", but this coverage would silently not happen. Searching progs/ for a zero-argument global subprog returning a constant matches only the two functions this patch adds. Every other plain-__noinline global subprog either takes pointer/scalar arguments or performs memory accesses (so memory(none) cannot be inferred), and files that need an opaque global subprog use __weak __noinline for this reason: progs/verifier_global_subprogs.c:__weak __noinline int noop_global_func() progs/summarization.c:__weak __noinline int noop_global_func() wq_global_acquire() survives only because it calls bpf_map_lookup_elem() through the constant function pointer from bpf_helper_defs.h, whose memory effects LLVM cannot infer. Would declaring wq_global_no_rcu() as __weak __noinline, or giving it an observable side effect, prevent the calls from being removed? This could be confirmed by checking llvm-objdump -d on the compiled .bpf.o to see if any `call -1` relocation to wq_global_no_rcu remains in wq_global_no_rcu_cb. > + > + val = bpf_map_lookup_elem(&wq_map, &key); > + if (!val) > + return 0; > + > + bpf_wq_init(&val->w, &wq_map, 0); > + bpf_wq_set_callback(&val->w, wq_global_no_rcu_cb, 0); > + return 0; > +} [ ... ] > @@ -179,3 +284,29 @@ int task_work_sleepable_prog(void *ctx) > bpf_task_work_schedule_resume(task, &val->tw, &task_work_map, task_work_cb); > return 0; > } > + > +static int task_work_global_rcu_cb(struct bpf_map *map, void *key, void *value) > +{ > + return wq_global_acquire(); > +} > + > +SEC("fentry/bpf_fentry_test1") > +__failure __msg("R1 must be a rcu pointer") > +int task_work_global_rcu_prog(void *ctx) > +{ > + struct task_work_elem *val; > + struct task_struct *task; > + int key = 0; > + > + val = bpf_map_lookup_elem(&task_work_map, &key); > + if (!val) > + return 0; > + > + task = bpf_get_current_task_btf(); > + if (!task) > + return 0; > + > + bpf_task_work_schedule_resume(task, &val->tw, &task_work_map, > + task_work_global_rcu_cb); > + return 0; > +} --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33943332260 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v1 0/2] Fix global subprog verification context 2026-09-05 3:40 [PATCH bpf-next v1 0/2] Fix global subprog verification context Kumar Kartikeya Dwivedi 2026-09-05 3:40 ` [PATCH bpf-next v1 1/2] bpf: Verify global subprogs in each sleepability context Kumar Kartikeya Dwivedi 2026-09-05 3:40 ` [PATCH bpf-next v1 2/2] selftests/bpf: Test global subprog callback contexts Kumar Kartikeya Dwivedi @ 2026-09-05 4:33 ` Kumar Kartikeya Dwivedi 2 siblings, 0 replies; 7+ messages in thread From: Kumar Kartikeya Dwivedi @ 2026-09-05 4:33 UTC (permalink / raw) To: bpf Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Eduard Zingerman, Emil Tsalapatis, Nicholas Carlini, kkd, kernel-team On Sat Sep 5, 2026 at 5:40 AM CEST, Kumar Kartikeya Dwivedi wrote: > Global subprogs can be invoked in both sleepable and non-sleepable > contexts, but are verified based on context assumptions for the program > type as a whole and not the precise context in which they are invoked. > Nicholas identified that this can cause verifier's safety assumptions to > be violated. Implement a fix by performing verification twice but only > if different contexts are exercised, and add selftests to ensure coverage. > > Kumar Kartikeya Dwivedi (2): > bpf: Verify global subprogs in each sleepability context > selftests/bpf: Test global subprog callback contexts > CI failure in GCC-BPF run looks unrelated. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-05 4:39 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-05 3:40 [PATCH bpf-next v1 0/2] Fix global subprog verification context Kumar Kartikeya Dwivedi 2026-09-05 3:40 ` [PATCH bpf-next v1 1/2] bpf: Verify global subprogs in each sleepability context Kumar Kartikeya Dwivedi 2026-09-05 3:54 ` sashiko-bot 2026-09-05 4:39 ` bot+bpf-ci 2026-09-05 3:40 ` [PATCH bpf-next v1 2/2] selftests/bpf: Test global subprog callback contexts Kumar Kartikeya Dwivedi 2026-09-05 4:39 ` bot+bpf-ci 2026-09-05 4:33 ` [PATCH bpf-next v1 0/2] Fix global subprog verification context Kumar Kartikeya Dwivedi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox