Re: [PATCH bpf-next v2 07/11] bpf: Introduce REF_TYPE_FRAME in the verifier

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf: UAF due to dropped parent_id in dynptr_from_mem for frame-scoped args
--

commit 19d3a1bd15552653b24ece884c759a788c6abaf9
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

bpf: Introduce REF_TYPE_FRAME in the verifier

This commit introduces REF_TYPE_FRAME to track references owned by a callee
frame. It allows a set_callee_state_fn to anchor an argument to a reference
which is dropped when the frame is popped.

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index c51083c761cf2..54a4f440c33b3 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -272,15 +272,13 @@ struct bpf_stack_state {
>  };
>  
>  struct bpf_reference_state {
> -	/* Each reference object has a type. Ensure REF_TYPE_PTR is zero to
> -	 * default to pointer reference on zero initialization of a state.
> -	 */
>  	enum ref_state_type {
>  		REF_TYPE_PTR		= (1 << 1),
>  		REF_TYPE_IRQ		= (1 << 2),
>  		REF_TYPE_LOCK		= (1 << 3),
>  		REF_TYPE_RES_LOCK 	= (1 << 4),
>  		REF_TYPE_RES_LOCK_IRQ	= (1 << 5),
> +		REF_TYPE_FRAME		= (1 << 6),
                ^^^^^^^^^^^^^^

[Severity: Critical]
Does this new reference type get ignored when the verifier checks if a register
is referenced?

In kernel/bpf/verifier.c, find_reference_state() explicitly skips any
reference type other than REF_TYPE_PTR:

kernel/bpf/verifier.c:find_reference_state() {
...
	for (i = 0; i < state->acquired_refs; i++) {
		if (state->refs[i].type != REF_TYPE_PTR)
			continue;
		if (state->refs[i].id == id)
			return true;
	}
...
}

Because find_reference_state() ignores REF_TYPE_FRAME, reg_is_referenced()
returns false for frame-scoped arguments.

Could this lead to a regression if a callback passes a frame-scoped reference
(like a PTR_TO_MAP_KEY array key or ringbuf dynptr slice) to a constructor
like bpf_dynptr_from_mem() to create a local dynptr?

When check_helper_call() processes the constructor, reg_is_referenced()
returns false, which leaves meta->ref_obj.id as 0:

kernel/bpf/verifier.c:check_helper_call() {
...
	if (reg_is_referenced(env, reg))
		update_ref_obj(&meta->ref_obj, reg);
...
}

Later, mark_stack_slots_dynptr() sets the dynptr's parent_id to this 0:

kernel/bpf/verifier.c:mark_stack_slots_dynptr() {
...
		/* Track parent's id if the parent is a referenced object */
		parent_id = ref_obj->id;
...
}

This strips the dynptr of its frame-scoped lifetime tie. When the callback
exits, release_frame_references() won't invalidate the dynptr because its
parent_id is 0.

Can the caller then extract a pointer from the dynptr using bpf_dynptr_data()
and access memory after the callback frame is destroyed?

Is it possible that pointer cast functions like bpf_skc_to_tcp_sock() also
suffer from the exact same logic flaw because they rely on
find_reference_state(..., meta.ref_obj.id)?

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