Re: [PATCH bpf-next v8 07/22] bpf: Follow an unwind to its landing pad in the verifier

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] bpf: verifier skips unwind paths when global subprog might throw
- [Critical] bpf: missing restriction for bpf_unwind in callbacks leads to verifier divergence
--

commit d554473b8011fe90d6322a255220e23a4d7500d5
Author: Yonghong Song <yonghong.song@linux.dev>
Subject: bpf: Follow an unwind to its landing pad in the verifier

This patch updates the BPF verifier to follow unwind operations to their
landing pads. It handles how an unwind rewrites return addresses of frames
and evaluates the proper precision backtracking.

> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -19126,6 +19145,127 @@ enum {
>  	INSN_IDX_UPDATED = 2,
>  };
>  
> +/*
> + * The current frame is leaving through an unwind. Its caller's saved return
> + * address now points at the pad covering the call, or, with none, at the
> + * caller's epilogue, and so on down. Follow that from the state the frame
> + * leaves in -- anything it wrote into its callers' stacks included -- to the
> + * first pad, or to the main program's frame returning.
> + */
> +static int unwind_frames(struct bpf_verifier_env *env, bool *do_print_state)
> +{
> +	struct bpf_verifier_state *state = env->cur_state;
> +	u32 frameno = state->curframe;
> +	struct bpf_func_state *callee, *caller;
> +	int err, pad;
> +
> +	while (state->curframe) {
> +		callee = cur_func(env);
> +		caller = state->frame[state->curframe - 1];
> +		pad = bpf_exc_pad_of_call(env, callee->callsite);
> +		/* The caller is at its call now, not at this frame's insn. */
> +		state->insn_idx = callee->callsite;
> +		account_processed_insns(env, callee, caller);
> +		free_func_state(callee);
> +		state->frame[state->curframe--] = NULL;
> +		invalidate_outgoing_stack_args(env, caller);
> +		if (pad < 0)
> +			continue;

[Severity: Critical]
If a callback subprogram (such as one passed to bpf_loop()) calls
bpf_unwind(), does this loop incorrectly pop frames in the verifier state past
the C helper?

It appears bpf_unwind() and bpf_unwind_resume() lack the callback restriction
that bpf_throw() has in do_check_subprogs().

At runtime, the exception unwinder stops unwinding when it encounters the
non-BPF C helper frame and returns. The callback returns 0, the C helper
resumes, and it returns normally to the BPF program.

However, if the verifier simulates an unwind through the C helper to the
caller's landing pad, it marks the normal execution path as dead code
and replaces it with a ja -1.

Could this divergence between verifier state and runtime execution cause an
infinite loop and hang the kernel?

[ ... ]
> @@ -19380,13 +19520,32 @@ static int do_check_insn(struct bpf_verifier_env *env, bool *do_print_state)
>  					return -EINVAL;
>  				}
>  			}
> +			if (bpf_is_unwind_kfunc(insn) || bpf_is_unwind_resume_kfunc(insn)) {
> +				err = check_kfunc_allowed_only(env, insn, env->insn_idx);
> +				if (err)
> +					return err;
> +				if (bpf_is_unwind_kfunc(insn))
> +					return process_bpf_unwind(env, &env->insn_idx,
> +								  do_print_state);
> +				/*
> +				 * The fixups lower this to 'r0 = 0; exit', and
> +				 * the unwind goes on below this frame.
> +				 */
> +				return unwind_frames(env, do_print_state);
> +			}
>  			mark_reg_scratched(env, BPF_REG_0);
>  			if (bpf_in_stack_arg_cnt(&env->subprog_info[cur_func(env)->subprogno]))
>  				cur_func(env)->no_stack_arg_load = true;
>  			if (bpf_is_callx(insn))
>  				return check_func_callx(env, insn, &env->insn_idx);
> -			if (insn->src_reg == BPF_PSEUDO_CALL)
> -				return check_func_call(env, insn, &env->insn_idx);
> +			if (insn->src_reg == BPF_PSEUDO_CALL) {
> +				int call_idx = env->insn_idx;
> +
> +				err = check_func_call(env, insn, &env->insn_idx);
> +				if (err)
> +					return err;
> +				return unwind_out_of_global_call(env, call_idx, do_print_state);
> +			}

[Severity: Critical]
If check_func_call() processes a global subprogram that is marked with both
might_throw and might_unwind, it evaluates might_throw, calls
process_bpf_exit_full(), and returns PROCESS_BPF_EXIT.

Does this cause do_check_insn() to return early here with err = 1, completely
bypassing the unwind_out_of_global_call() logic?

If bypassed, the landing pad associated with the call is never verified and
is replaced with a ja -1 (dead code). At runtime, if the global subprogram
calls bpf_unwind(), the exception unwinder routes execution to this unverified
landing pad. 

Can this result in hitting the ja -1 infinite loop and hanging the kernel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001133006.1335369-1-yonghong.song@linux.dev?part=7