Re: [PATCH bpf-next v3 3/5] bpf: Add a __derives_ret kfunc argument annotation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf: check_func_arg() fails to propagate parent_id into meta->ref_obj
--

commit 3a36b9b9ff614d7729738b098cb55d91d05fb314
Author: Justin Suess <utilityemal77@gmail.com>

bpf: Add a __derives_ret kfunc argument annotation

This commit adds a __derives_ret parameter name suffix for kfunc pointer
arguments whose lifetime bounds the returned pointer. When a kfunc with a
__derives_ret argument is called, the verifier propagates the argument's
reference lifetime (via parent_id) or RCU protection (via MEM_RCU) to the
returned register.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 4a32875d70778..5a13b5ef688b7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -9848,6 +9856,13 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, u32 slot, u32 p
>  	if (reg_is_referenced(env, reg))
>  		update_ref_obj(&meta->ref_obj, reg);
>  
> +	/* The return of a __derives_ret kfunc lives exactly as long as this argument. */
> +	if (btf_arg && is_kfunc_arg_derives_ret(meta->btf, btf_arg)) {
> +		meta->derives_ret.found = true;
> +		meta->derives_ret.rcu = is_rcu_reg(reg);
> +		meta->derives_ret.parent_id = reg_lifetime_id(env, reg);
> +	}

[Severity: Critical]
When a pointer returned by a __derives_ret kfunc is kept alive by a parent
reference (where reg->id is not in cur_state->refs, but reg->parent_id holds
the active reference ID), should check_func_arg() and update_ref_obj() also
propagate reg_lifetime_id(env, reg) into meta->ref_obj?

Currently, in kernel/bpf/verifier.c:check_func_arg(), update_ref_obj() is
only called when reg_is_referenced(env, reg) is true, and update_ref_obj()
records reg->id rather than reg_lifetime_id(env, reg):

kernel/bpf/verifier.c:update_ref_obj() {
	ref_obj->id = reg->id;
	ref_obj->parent_id = reg->parent_id;
	ref_obj->cnt++;
}

If a BPF program acquires a referenced object, passes it to a __derives_ret
kfunc to obtain a borrowed PTR_TO_BTF_ID | PTR_TRUSTED pointer with id == 0
and parent_id != 0, and then passes that derived pointer to another helper
or kfunc that relies on meta->ref_obj.id, meta->ref_obj.id remains 0:

1. In check_func_arg() -> process_dynptr_func() -> mark_stack_slots_dynptr()
   (for example, via bpf_dynptr_from_file() or bpf_dynptr_from_skb()),
   mark_stack_slots_dynptr() sets:

	parent_id = ref_obj->id;
	if (dynptr_type_referenced(type)) {
		id = acquire_reference(env, insn_idx, parent_id);

   Because ref_obj->id is 0, the dynptr and any child reference it acquires
   are created with parent_id == 0 instead of the underlying reference ID,
   so releasing the original reference neither rejects an unreleased file
   dynptr nor invalidates the dynptr or its slices.

2. In check_kfunc_call(), for a non-__derives_ret kfunc returning PTR_TO_MEM:

	if (meta.derives_ret.found) {
		regs[BPF_REG_0].parent_id = meta.derives_ret.parent_id;
	} else if (meta.ref_obj.id) {
		err = validate_ref_obj(env, &meta.ref_obj);
		if (err)
			return err;
		regs[BPF_REG_0].parent_id = meta.ref_obj.id;
	}

   When passed a __derives_ret-derived pointer, meta.ref_obj.id is 0, leaving
   regs[BPF_REG_0].parent_id as 0 (and bypassing validate_ref_obj() if
   another referenced argument was also passed).

3. In check_helper_call() for socket pointer-casting helpers
   (is_ptr_cast_function()), meta.ref_obj.id is 0, so the cast pointer is
   not bound to the original reference.

Can this allow derived dynptrs, memory pointers, or cast socket pointers to
outlive release_reference() on the original acquired object and cause a
use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023212041.884996-1-utilityemal77@gmail.com?part=3