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. Since a global subprogram may now be verified twice, accumulate both passes in subprog_info[].insns_total. This makes BPF_LOG_STATS and per-subprogram veristat output account for both contexts instead of reporting only the last pass. 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 Suggested-by: Nicholas Carlini Signed-off-by: Kumar Kartikeya Dwivedi --- include/linux/bpf.h | 5 +-- kernel/bpf/verifier.c | 77 ++++++++++++++++++++++++++----------------- 2 files changed, 50 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..f759a020c8a5 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,55 @@ 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 +19817,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