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

[Yonghong Song <yonghong.song@linux.dev>]

On 10/1/26 6:53 AM, sashiko-bot@kernel.org wrote:
> 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?

I cannot figure out a condition to have misaligned stack (i.e. using r9)
like the above condition.

I think the current condition should be okay.

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