Re: [PATCH bpf-next v8 10/22] bpf: Do not use a private stack for a program that can unwind

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: callee-saved register corruption via bpf_throw without cleanup
--

commit 9d2c0e8faeb024a4647980c48ca2caa933b90b1c
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Do not use a private stack for a program that can unwind

This commit disables private stacks for BPF programs that can unwind
because an unwind skips the pop instruction corresponding to the push r9
emitted around calls. This forces NO_PRIV_STACK in check_max_stack_depth()
for programs with cleanup records or that may unwind.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f3ed68960d70d..488ceb9dae1b6 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5769,6 +5769,17 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
>  		}
>  	}
>  
> +	/*
> +	 * A private stack keeps its frame pointer in %r9 on x86-64, restored
> +	 * by a pop after the call that an unwind skips. A frame resumed at a
> +	 * pad then addresses its stack through a stale pointer, and a frame
> +	 * sent to its epilogue instead pops its callee-saved registers one
> +	 * slot off. Refuse a private stack for any program that can unwind,
> +	 * on every arch for now.
> +	 */
> +	if (env->cleanup_info_cnt || bpf_prog_may_unwind(env))
> +		priv_stack_mode = NO_PRIV_STACK;

[Severity: High]
Does this logic miss programs that call bpf_throw() without any cleanup
records?

If a privileged program uses private stacks and calls bpf_throw() without
acquiring any resources requiring cleanup, cleanup_info_cnt will be 0.

Looking at bpf_prog_may_unwind() in kernel/bpf/exception.c, it only checks
might_unwind:

kernel/bpf/exception.c:bpf_prog_may_unwind() {
	u32 i;

	for (i = 0; i < env->subprog_cnt; i++)
		if (env->subprog_info[i].might_unwind)
			return true;
	return false;
}

Since bpf_throw() only sets might_throw, not might_unwind, it appears
bpf_prog_may_unwind() will return false.

Could this bypass the NO_PRIV_STACK enforcement and lead to the exception
callback epilogue popping the misaligned stack (shifted by the skipped
push r9) into the kernel's callee-saved registers?

> +
>  	if (priv_stack_mode == PRIV_STACK_UNKNOWN)
>  		priv_stack_mode = bpf_enable_priv_stack(env->prog);
>

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