Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

bpf: fix the return value of push_stack

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 <a.s.protopopov@gmail.com>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20251019202145.3944697-2-a.s.protopopov@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

authored by

Anton Protopopov and committed by
Alexei Starovoitov
6ea5fc92 96d31dff

+40 -40
+40 -40
kernel/bpf/verifier.c
··· 2109 2109 2110 2110 elem = kzalloc(sizeof(struct bpf_verifier_stack_elem), GFP_KERNEL_ACCOUNT); 2111 2111 if (!elem) 2112 - return NULL; 2112 + return ERR_PTR(-ENOMEM); 2113 2113 2114 2114 elem->insn_idx = insn_idx; 2115 2115 elem->prev_insn_idx = prev_insn_idx; ··· 2119 2119 env->stack_size++; 2120 2120 err = copy_verifier_state(&elem->st, cur); 2121 2121 if (err) 2122 - return NULL; 2122 + return ERR_PTR(-ENOMEM); 2123 2123 elem->st.speculative |= speculative; 2124 2124 if (env->stack_size > BPF_COMPLEXITY_LIMIT_JMP_SEQ) { 2125 2125 verbose(env, "The sequence of %d jumps is too complex.\n", 2126 2126 env->stack_size); 2127 - return NULL; 2127 + return ERR_PTR(-E2BIG); 2128 2128 } 2129 2129 if (elem->st.parent) { 2130 2130 ++elem->st.parent->branches; ··· 2919 2919 2920 2920 elem = kzalloc(sizeof(struct bpf_verifier_stack_elem), GFP_KERNEL_ACCOUNT); 2921 2921 if (!elem) 2922 - return NULL; 2922 + return ERR_PTR(-ENOMEM); 2923 2923 2924 2924 elem->insn_idx = insn_idx; 2925 2925 elem->prev_insn_idx = prev_insn_idx; ··· 2931 2931 verbose(env, 2932 2932 "The sequence of %d jumps is too complex for async cb.\n", 2933 2933 env->stack_size); 2934 - return NULL; 2934 + return ERR_PTR(-E2BIG); 2935 2935 } 2936 2936 /* Unlike push_stack() do not copy_verifier_state(). 2937 2937 * The caller state doesn't matter. ··· 2942 2942 elem->st.in_sleepable = is_sleepable; 2943 2943 frame = kzalloc(sizeof(*frame), GFP_KERNEL_ACCOUNT); 2944 2944 if (!frame) 2945 - return NULL; 2945 + return ERR_PTR(-ENOMEM); 2946 2946 init_func_state(env, frame, 2947 2947 BPF_MAIN_FUNC /* callsite */, 2948 2948 0 /* frameno within this callchain */, ··· 9045 9045 prev_st = find_prev_entry(env, cur_st->parent, insn_idx); 9046 9046 /* branch out active iter state */ 9047 9047 queued_st = push_stack(env, insn_idx + 1, insn_idx, false); 9048 - if (!queued_st) 9049 - return -ENOMEM; 9048 + if (IS_ERR(queued_st)) 9049 + return PTR_ERR(queued_st); 9050 9050 9051 9051 queued_iter = get_iter_from_state(queued_st, meta); 9052 9052 queued_iter->iter.state = BPF_ITER_STATE_ACTIVE; ··· 10616 10616 async_cb = push_async_cb(env, env->subprog_info[subprog].start, 10617 10617 insn_idx, subprog, 10618 10618 is_async_cb_sleepable(env, insn)); 10619 - if (!async_cb) 10620 - return -EFAULT; 10619 + if (IS_ERR(async_cb)) 10620 + return PTR_ERR(async_cb); 10621 10621 callee = async_cb->frame[0]; 10622 10622 callee->async_entry_cnt = caller->async_entry_cnt + 1; 10623 10623 ··· 10633 10633 * proceed with next instruction within current frame. 10634 10634 */ 10635 10635 callback_state = push_stack(env, env->subprog_info[subprog].start, insn_idx, false); 10636 - if (!callback_state) 10637 - return -ENOMEM; 10636 + if (IS_ERR(callback_state)) 10637 + return PTR_ERR(callback_state); 10638 10638 10639 10639 err = setup_func_entry(env, subprog, insn_idx, set_callee_state_cb, 10640 10640 callback_state); ··· 13859 13859 struct bpf_reg_state *regs; 13860 13860 13861 13861 branch = push_stack(env, env->insn_idx + 1, env->insn_idx, false); 13862 - if (!branch) { 13862 + if (IS_ERR(branch)) { 13863 13863 verbose(env, "failed to push state for failed lock acquisition\n"); 13864 - return -ENOMEM; 13864 + return PTR_ERR(branch); 13865 13865 } 13866 13866 13867 13867 regs = branch->frame[branch->curframe]->regs; ··· 14316 14316 bool mask_to_left; 14317 14317 }; 14318 14318 14319 - static struct bpf_verifier_state * 14320 - sanitize_speculative_path(struct bpf_verifier_env *env, 14321 - const struct bpf_insn *insn, 14322 - u32 next_idx, u32 curr_idx) 14319 + static int sanitize_speculative_path(struct bpf_verifier_env *env, 14320 + const struct bpf_insn *insn, 14321 + u32 next_idx, u32 curr_idx) 14323 14322 { 14324 14323 struct bpf_verifier_state *branch; 14325 14324 struct bpf_reg_state *regs; 14326 14325 14327 14326 branch = push_stack(env, next_idx, curr_idx, true); 14328 - if (branch && insn) { 14327 + if (!IS_ERR(branch) && insn) { 14329 14328 regs = branch->frame[branch->curframe]->regs; 14330 14329 if (BPF_SRC(insn->code) == BPF_K) { 14331 14330 mark_reg_unknown(env, regs, insn->dst_reg); ··· 14333 14334 mark_reg_unknown(env, regs, insn->src_reg); 14334 14335 } 14335 14336 } 14336 - return branch; 14337 + return PTR_ERR_OR_ZERO(branch); 14337 14338 } 14338 14339 14339 14340 static int sanitize_ptr_alu(struct bpf_verifier_env *env, ··· 14352 14353 u8 opcode = BPF_OP(insn->code); 14353 14354 u32 alu_state, alu_limit; 14354 14355 struct bpf_reg_state tmp; 14355 - bool ret; 14356 14356 int err; 14357 14357 14358 14358 if (can_skip_alu_sanitation(env, insn)) ··· 14424 14426 tmp = *dst_reg; 14425 14427 copy_register_state(dst_reg, ptr_reg); 14426 14428 } 14427 - ret = sanitize_speculative_path(env, NULL, env->insn_idx + 1, 14428 - env->insn_idx); 14429 - if (!ptr_is_dst_reg && ret) 14429 + err = sanitize_speculative_path(env, NULL, env->insn_idx + 1, env->insn_idx); 14430 + if (err < 0) 14431 + return REASON_STACK; 14432 + if (!ptr_is_dst_reg) 14430 14433 *dst_reg = tmp; 14431 - return !ret ? REASON_STACK : 0; 14434 + return 0; 14432 14435 } 14433 14436 14434 14437 static void sanitize_mark_insn_seen(struct bpf_verifier_env *env) ··· 16749 16750 16750 16751 /* branch out 'fallthrough' insn as a new state to explore */ 16751 16752 queued_st = push_stack(env, idx + 1, idx, false); 16752 - if (!queued_st) 16753 - return -ENOMEM; 16753 + if (IS_ERR(queued_st)) 16754 + return PTR_ERR(queued_st); 16754 16755 16755 16756 queued_st->may_goto_depth++; 16756 16757 if (prev_st) ··· 16828 16829 * the fall-through branch for simulation under speculative 16829 16830 * execution. 16830 16831 */ 16831 - if (!env->bypass_spec_v1 && 16832 - !sanitize_speculative_path(env, insn, *insn_idx + 1, 16833 - *insn_idx)) 16834 - return -EFAULT; 16832 + if (!env->bypass_spec_v1) { 16833 + err = sanitize_speculative_path(env, insn, *insn_idx + 1, *insn_idx); 16834 + if (err < 0) 16835 + return err; 16836 + } 16835 16837 if (env->log.level & BPF_LOG_LEVEL) 16836 16838 print_insn_state(env, this_branch, this_branch->curframe); 16837 16839 *insn_idx += insn->off; ··· 16842 16842 * program will go. If needed, push the goto branch for 16843 16843 * simulation under speculative execution. 16844 16844 */ 16845 - if (!env->bypass_spec_v1 && 16846 - !sanitize_speculative_path(env, insn, 16847 - *insn_idx + insn->off + 1, 16848 - *insn_idx)) 16849 - return -EFAULT; 16845 + if (!env->bypass_spec_v1) { 16846 + err = sanitize_speculative_path(env, insn, *insn_idx + insn->off + 1, 16847 + *insn_idx); 16848 + if (err < 0) 16849 + return err; 16850 + } 16850 16851 if (env->log.level & BPF_LOG_LEVEL) 16851 16852 print_insn_state(env, this_branch, this_branch->curframe); 16852 16853 return 0; ··· 16868 16867 return err; 16869 16868 } 16870 16869 16871 - other_branch = push_stack(env, *insn_idx + insn->off + 1, *insn_idx, 16872 - false); 16873 - if (!other_branch) 16874 - return -EFAULT; 16870 + other_branch = push_stack(env, *insn_idx + insn->off + 1, *insn_idx, false); 16871 + if (IS_ERR(other_branch)) 16872 + return PTR_ERR(other_branch); 16875 16873 other_branch_regs = other_branch->frame[other_branch->curframe]->regs; 16876 16874 16877 16875 if (BPF_SRC(insn->code) == BPF_X) {