From: Anton Protopopov commit 6ea5fc92a0fc1cde976cb701db2c1dba4dcab7cf upstream. In [1] Eduard mentioned that on push_stack failure verifier code should return -ENOMEM instead of -EFAULT. After checking with the other call sites I've found that code randomly returns either -ENOMEM or -EFAULT. This patch unifies the return values for the push_stack (and similar push_async_cb) functions such that error codes are always assigned properly. [1] https://lore.kernel.org/bpf/20250615085943.3871208-1-a.s.protopopov@gmail.com Signed-off-by: Anton Protopopov Acked-by: Eduard Zingerman Link: https://lore.kernel.org/r/20251019202145.3944697-2-a.s.protopopov@gmail.com Signed-off-by: Alexei Starovoitov [ Ajay: Modified to apply on v6.12. The check_kfunc_call hunk for a failed lock acquisition was dropped: that code path does not exist in v6.12. ] Signed-off-by: Ajay Kaher --- kernel/bpf/verifier.c | 88 +++++++++++++++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 38 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 64a6ec8eb847..4c439ab978a7 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1700,8 +1700,10 @@ static struct bpf_verifier_state *push_stack(struct bpf_verifier_env *env, int err; elem = kzalloc(sizeof(struct bpf_verifier_stack_elem), GFP_KERNEL); - if (!elem) + if (!elem) { + err = -ENOMEM; goto err; + } elem->insn_idx = insn_idx; elem->prev_insn_idx = prev_insn_idx; @@ -1710,12 +1712,15 @@ static struct bpf_verifier_state *push_stack(struct bpf_verifier_env *env, env->head = elem; env->stack_size++; err = copy_verifier_state(&elem->st, cur); - if (err) + if (err) { + err = -ENOMEM; goto err; + } elem->st.speculative |= speculative; if (env->stack_size > BPF_COMPLEXITY_LIMIT_JMP_SEQ) { verbose(env, "The sequence of %d jumps is too complex.\n", env->stack_size); + err = -E2BIG; goto err; } if (elem->st.parent) { @@ -1736,7 +1741,7 @@ static struct bpf_verifier_state *push_stack(struct bpf_verifier_env *env, env->cur_state = NULL; /* pop all elements and return */ while (!pop_stack(env, NULL, NULL, false)); - return NULL; + return ERR_PTR(err); } #define CALLER_SAVED_REGS 6 @@ -2542,10 +2547,13 @@ static struct bpf_verifier_state *push_async_cb(struct bpf_verifier_env *env, { struct bpf_verifier_stack_elem *elem; struct bpf_func_state *frame; + int err; elem = kzalloc(sizeof(struct bpf_verifier_stack_elem), GFP_KERNEL); - if (!elem) + if (!elem) { + err = -ENOMEM; goto err; + } elem->insn_idx = insn_idx; elem->prev_insn_idx = prev_insn_idx; @@ -2557,6 +2565,7 @@ static struct bpf_verifier_state *push_async_cb(struct bpf_verifier_env *env, verbose(env, "The sequence of %d jumps is too complex for async cb.\n", env->stack_size); + err = -E2BIG; goto err; } /* Unlike push_stack() do not copy_verifier_state(). @@ -2572,8 +2581,10 @@ static struct bpf_verifier_state *push_async_cb(struct bpf_verifier_env *env, elem->st.insn_hist_start = env->cur_state->insn_hist_end; elem->st.insn_hist_end = elem->st.insn_hist_start; frame = kzalloc(sizeof(*frame), GFP_KERNEL); - if (!frame) + if (!frame) { + err = -ENOMEM; goto err; + } init_func_state(env, frame, BPF_MAIN_FUNC /* callsite */, 0 /* frameno within this callchain */, @@ -2585,7 +2596,7 @@ static struct bpf_verifier_state *push_async_cb(struct bpf_verifier_env *env, env->cur_state = NULL; /* pop all elements and return */ while (!pop_stack(env, NULL, NULL, false)); - return NULL; + return ERR_PTR(err); } @@ -8471,8 +8482,8 @@ static int process_iter_next_call(struct bpf_verifier_env *env, int insn_idx, prev_st = find_prev_entry(env, cur_st->parent, insn_idx); /* branch out active iter state */ queued_st = push_stack(env, insn_idx + 1, insn_idx, false); - if (!queued_st) - return -ENOMEM; + if (IS_ERR(queued_st)) + return PTR_ERR(queued_st); queued_iter = get_iter_from_state(queued_st, meta); queued_iter->iter.state = BPF_ITER_STATE_ACTIVE; @@ -9947,8 +9958,8 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins async_cb = push_async_cb(env, env->subprog_info[subprog].start, insn_idx, subprog, is_bpf_wq_set_callback_impl_kfunc(insn->imm)); - if (!async_cb) - return -EFAULT; + if (IS_ERR(async_cb)) + return PTR_ERR(async_cb); callee = async_cb->frame[0]; callee->async_entry_cnt = caller->async_entry_cnt + 1; @@ -9964,8 +9975,8 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins * proceed with next instruction within current frame. */ callback_state = push_stack(env, env->subprog_info[subprog].start, insn_idx, false); - if (!callback_state) - return -ENOMEM; + if (IS_ERR(callback_state)) + return PTR_ERR(callback_state); err = setup_func_entry(env, subprog, insn_idx, set_callee_state_cb, callback_state); @@ -13246,16 +13257,15 @@ struct bpf_sanitize_info { bool mask_to_left; }; -static struct bpf_verifier_state * -sanitize_speculative_path(struct bpf_verifier_env *env, - const struct bpf_insn *insn, - u32 next_idx, u32 curr_idx) +static int sanitize_speculative_path(struct bpf_verifier_env *env, + const struct bpf_insn *insn, + u32 next_idx, u32 curr_idx) { struct bpf_verifier_state *branch; struct bpf_reg_state *regs; branch = push_stack(env, next_idx, curr_idx, true); - if (branch && insn) { + if (!IS_ERR(branch) && insn) { regs = branch->frame[branch->curframe]->regs; if (BPF_SRC(insn->code) == BPF_K) { mark_reg_unknown(env, regs, insn->dst_reg); @@ -13264,7 +13274,7 @@ sanitize_speculative_path(struct bpf_verifier_env *env, mark_reg_unknown(env, regs, insn->src_reg); } } - return branch; + return PTR_ERR_OR_ZERO(branch); } static int sanitize_ptr_alu(struct bpf_verifier_env *env, @@ -13283,7 +13293,6 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env, u8 opcode = BPF_OP(insn->code); u32 alu_state, alu_limit; struct bpf_reg_state tmp; - bool ret; int err; if (can_skip_alu_sanitation(env, insn)) @@ -13356,11 +13365,12 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env, tmp = *dst_reg; copy_register_state(dst_reg, ptr_reg); } - ret = sanitize_speculative_path(env, NULL, env->insn_idx + 1, - env->insn_idx); - if (!ptr_is_dst_reg && ret) + err = sanitize_speculative_path(env, NULL, env->insn_idx + 1, env->insn_idx); + if (err < 0) + return REASON_STACK; + if (!ptr_is_dst_reg) *dst_reg = tmp; - return !ret ? REASON_STACK : 0; + return 0; } static void sanitize_mark_insn_seen(struct bpf_verifier_env *env) @@ -15672,8 +15682,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, /* branch out 'fallthrough' insn as a new state to explore */ queued_st = push_stack(env, idx + 1, idx, false); - if (!queued_st) - return -ENOMEM; + if (IS_ERR(queued_st)) + return PTR_ERR(queued_st); queued_st->may_goto_depth++; if (prev_st) @@ -15751,10 +15761,11 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, * the fall-through branch for simulation under speculative * execution. */ - if (!env->bypass_spec_v1 && - !sanitize_speculative_path(env, insn, *insn_idx + 1, - *insn_idx)) - return -EFAULT; + if (!env->bypass_spec_v1) { + err = sanitize_speculative_path(env, insn, *insn_idx + 1, *insn_idx); + if (err < 0) + return err; + } if (env->log.level & BPF_LOG_LEVEL) print_insn_state(env, this_branch->frame[this_branch->curframe]); *insn_idx += insn->off; @@ -15764,11 +15775,12 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, * program will go. If needed, push the goto branch for * simulation under speculative execution. */ - if (!env->bypass_spec_v1 && - !sanitize_speculative_path(env, insn, - *insn_idx + insn->off + 1, - *insn_idx)) - return -EFAULT; + if (!env->bypass_spec_v1) { + err = sanitize_speculative_path(env, insn, *insn_idx + insn->off + 1, + *insn_idx); + if (err < 0) + return err; + } if (env->log.level & BPF_LOG_LEVEL) print_insn_state(env, this_branch->frame[this_branch->curframe]); return 0; @@ -15791,8 +15803,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, other_branch = push_stack(env, *insn_idx + insn->off + 1, *insn_idx, false); - if (!other_branch) - return -EFAULT; + if (IS_ERR(other_branch)) + return PTR_ERR(other_branch); other_branch_regs = other_branch->frame[other_branch->curframe]->regs; if (BPF_SRC(insn->code) == BPF_X) { @@ -16129,8 +16141,8 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn) mark_reg_scratched(env, BPF_REG_0); branch = push_stack(env, env->insn_idx + 1, env->insn_idx, false); - if (!branch) - return -EFAULT; + if (IS_ERR(branch)) + return PTR_ERR(branch); mark_reg_known_zero(env, regs, BPF_REG_0); err = prepare_func_exit(env, &env->insn_idx); if (err) -- 2.53.0