Re: [PATCH bpf-next v1 2/2] bpf: Rely on callback frame initialization defaults

[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: missing callback dynptr invalidation in
  prepare_func_exit() [1]
--

commit 468e6b1f0288b436ee1abb6fdbf7f6a231d967b2
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

bpf: Rely on callback frame initialization defaults

This commit removes redundant NOT_INIT register resets and default return
range assignments from BPF verifier callback state setters, relying on the
defaults already set by init_func_state().

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 389488e4e7a74..831a9c0f8c458 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -12126,15 +12110,9 @@ static int set_user_ringbuf_callback_state(struct bpf_verifier_env *env,
>  	 *			  callback_ctx, u64 flags);
>  	 * callback_fn(const struct bpf_dynptr_t* dynptr, void *callback_ctx);
>  	 */
> -	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_0]);
>  	mark_dynptr_cb_reg(env, &callee->regs[BPF_REG_1], BPF_DYNPTR_TYPE_LOCAL);
>  	callee->regs[BPF_REG_2] = caller->regs[BPF_REG_3];
>  
> -	/* unused */
> -	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_3]);
> -	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
> -	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_5]);
> -
>  	callee->in_callback_fn = true;
>  	callee->callback_ret_range = retval_range(0, 1);
>  	return 0;

[Severity: Critical]
This problem wasn't introduced by this patch [1], but can the
callback dynptr in R1 or a slice derived from it escape into a caller's
stack frame via R2 (callback_ctx)?

mark_dynptr_cb_reg() initializes R1 as a CONST_PTR_TO_DYNPTR with a fresh
id and parent_id set to 0.  When callback_ctx points into an outer stack
frame, check_stack_write_fixed_off() only blocks spilling PTR_TO_STACK
into a caller's frame, so the callback can spill R1, spill a PTR_TO_MEM
slice derived from R1, or clone R1 into the caller's stack via
bpf_dynptr_clone().

On callback return, prepare_func_exit() calls
destroy_dynptrs_in_stack_slots() only on the callee's own stack frame and
never invalidates references tied to R1's id in outer frames.  Because the
underlying struct bpf_dynptr_kern lives on the kernel C stack of
bpf_user_ringbuf_drain() and the sample is released after each callback
invocation, the escaped dynptr or slice remains accessible in the caller
after bpf_user_ringbuf_drain() returns, leading to kernel stack
use-after-return and use-after-release of user ring buffer samples.

[1] https://sashiko.dev/#/bug/linux-05b65197-996a-40dd-bf7b-7710de2720b4

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106232110.2867412-1-ihor.solodrai@linux.dev?part=2