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

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 11/6/26 4:48 PM, sashiko-bot@kernel.org wrote:
> 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)?

Yeah. This is exactly what is addressed by the series
"bpf: Scope callback arguments to their frame":
https://lore.kernel.org/bpf/20261106231157.2840635-1-ihor.solodrai@linux.dev/


> 
> 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
>