Re: [PATCH 6.6.y v2 2/3] bpf: Propagate referenced dynptr release errors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The commit fixes a kernel panic where a callback releasing a caller-acquired dynptr reference triggers a WARN_ON_ONCE, but leaves the identical vulnerability unfixed for BPF iterators in unmark_stack_slots_iter().
- [High] A kernel panic can be triggered via WARN_ON_ONCE(release_reference_state(...)) in mark_ptr_or_null_regs() when performing a NULL check on a caller's spilled PTR_OR_NULL register inside a callback.
--

commit 0666efa4fca54b7a83f6a6c69d44d1c775940a2e
Author: Amery Hung <ameryhung@gmail.com>
Subject: bpf: Propagate referenced dynptr release errors

This patch fixes a panic caused by a WARN_ON_ONCE in the verifier when a BPF
callback incorrectly attempts to release a referenced dynptr acquired by its
caller. It does this by correctly propagating the error returned from
release_reference() in unmark_stack_slots_dynptr().

Link: https://lore.kernel.org/r/20260529014936.2811085-6-ameryhung@gmail.com

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index ad679bc42427c..c9e3856bb59fd 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -1013,8 +1017,16 @@ static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_re
>  	 * 2) Any slices derived from this dynptr.
>  	 */
>  
> -	/* Invalidate any slices associated with this dynptr */
> -	WARN_ON_ONCE(release_reference(env, ref_obj_id));
> +	/* Invalidate any slices associated with this dynptr.
> +	 * release_reference() only walks registers that carry ref_obj_id;
> +	 * slices are PTR_TO_MEM with dynptr_id set and ref_obj_id left 0.
> +	 * Mirror destroy_if_dynptr_stack_slot(), which already invalidates
> +	 * slices by dynptr_id.
> +	 */
> +	err = release_reference(env, ref_obj_id);
> +	if (err)
> +		return err;

[Severity: High]
This isn't a bug introduced by this patch, but does the same vulnerability
still exist for BPF iterators in unmark_stack_slots_iter()?

If a caller allocates an iterator on its stack, passes a pointer to this stack
slot into a callback via bpf_loop(), and the callback invokes a destroy kfunc
(like bpf_iter_num_destroy()), the verifier will call:

kernel/bpf/verifier.c:unmark_stack_slots_iter() {
    ...
		if (i == 0)
			WARN_ON_ONCE(release_reference(env, st->ref_obj_id));
    ...
}

Because the reference belongs to the caller frame, release_reference() returns
-EINVAL, which triggers the WARN_ON_ONCE and panics the kernel if
panic_on_warn=1.

[Severity: High]
This is a pre-existing issue, but could a similar panic occur when performing
a NULL check on a caller's spilled PTR_OR_NULL register inside a callback?

If a caller acquires a reference-tracked pointer that could be NULL, spills it
to the stack, and passes a pointer to that stack slot to a callback, the
restored register preserves the caller's id and ref_obj_id.

When the callback performs a NULL check on this restored pointer:

kernel/bpf/verifier.c:mark_ptr_or_null_regs() {
    ...
	if (ref_obj_id && ref_obj_id == id && is_null)
		/* regs[regno] is in the " == NULL" branch.
		 * No one could have freed the reference state before
		 * doing the NULL check.
		 */
		WARN_ON_ONCE(release_reference_state(state, id));
    ...
}

Because the reference belongs to the caller frame, release_reference_state()
returns -EINVAL, triggering the warning.

> +	invalidate_slices_of_dynptr(env, state->stack[spi].spilled_ptr.id);
>  
>  	/* Invalidate any dynptr clones */
>  	for (i = 1; i < state->allocated_stack / BPF_REG_SIZE; i++) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829102618.105116-1-xyx2021@mail.ustc.edu.cn?part=2