Re: [PATCH bpf-next v1 07/29] bpf: Add one allocator for hidden stack slots

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] bpf: stack buffer overflow in ___bpf_prog_run() [1]
--

commit 12562841fbd23150e8e97550f85ed29a199cd037
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Add one allocator for hidden stack slots

This commit introduces a unified hidden stack slot allocator for
post-verification rewrites and converts may_goto, inlined bpf_loop(), and
epilogue context saving to use it.

> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 8597b443d5619..04d89113e5963 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
[ ... ]
> @@ -3512,9 +3596,15 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
>  	}
>  
>  	ret = bpf_patch_list_commit(env);
> +	if (ret)
> +		return ret;
> +
> +	/* The hidden slots get their values in the prologues too. */
> +	ret = bpf_emit_stack_inits(env);
>  	if (ret)
>  		return ret;
>  	prog = env->prog;
> +	prog->aux->stack_depth = subprogs[0].stack_depth;

[Severity: Critical]
This problem wasn't introduced by this patch, but can a tail-calling program
that fails JIT compilation overflow the kernel stack in ___bpf_prog_run()
[1]?

In bpf_do_misc_fixups(), when a BPF_FUNC_tail_call instruction is seen, the
verifier only sets prog->aux->stack_depth to MAX_BPF_STACK when
!bpf_allow_tail_call_in_subprogs(env) (and then overwrites
prog->aux->stack_depth with subprogs[0].stack_depth at the end of the
function):

kernel/bpf/fixups.c:bpf_do_misc_fixups() {
	...
		if (insn->imm == BPF_FUNC_tail_call) {
			prog->cb_access = 1;
			if (!bpf_allow_tail_call_in_subprogs(env))
				prog->aux->stack_depth = MAX_BPF_STACK;
	...
}

On architectures where bpf_allow_tail_call_in_subprogs() is true because
jit_requested is set, if JIT compilation later fails (for example, on memory
allocation failure) and falls back to the interpreter,
bpf_prog_select_interpreter() selects an interpreter function sized only for
the caller's unexpanded stack_depth.  When JMP_TAIL_CALL in
___bpf_prog_run() switches insn to a target program that uses a deeper stack
(up to MAX_BPF_STACK) without allocating a new C stack frame, the target
program writes past the caller's fixed-size stack array and corrupts the
kernel stack.

[1] https://sashiko.dev/#/bug/linux-57b05589-4ccc-4b7c-94c0-c3316f3932e4

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794399387.git.memxor@gmail.com?part=7