Re: [PATCH v1 bpf-next 13/18] bpf: Check the return values of fmod_ret programs on found targets

[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: use-after-free in check_helper_call() [1]
--

commit f5e3bd12bcf921a0d31fb8a40c17694ea46d4300
Author: Anton Protopopov <a.s.protopopov@gmail.com>

bpf: Check the return values of fmod_ret programs on found targets

This commit parses the "bpf_fmodret_retval:" BTF decl tag on fmod_ret
target functions and verifies that attached fmod_ret programs only return
0 or one of the specified error values, while disallowing tail calls and
freplace extensions on such programs.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1b58a1796a2e4..71fbb30e4d768 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13338,6 +13338,14 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>  	}
>  
>  	if (func_id == BPF_FUNC_tail_call) {
> +		/*
> +		 * The program which a tail call jumps to returns for this one,
> +		 * and a program array does not check the values it may return.
> +		 */
> +		if (env->prog->aux->fmodret_retvals) {
> +			verbose(env, "tail calls are not allowed in fmod_ret programs with restricted return values\n");
> +			return -EINVAL;
> +		}
>  		if (env->cur_state->curframe) {
>  			struct bpf_verifier_state *branch;
>  

[Severity: Critical]
This problem wasn't introduced by this patch, but in check_helper_call(),
can passing an RCU-protected or iterator-yielded pointer to a pointer-cast
helper such as bpf_skc_to_tcp_sock() drop its lifetime constraints on
regs[BPF_REG_0] [1]?

kernel/bpf/verifier.c:check_helper_call() {
	...
	if (is_ptr_cast_function(func_id) &&
	    find_reference_state(env->cur_state, meta.ref_obj.id)) {
		...
		regs[BPF_REG_0].type &= ~PTR_MAYBE_NULL;
		regs[BPF_REG_0].id = meta.ref_obj.id;
	}
	...
}

RET_PTR_TO_BTF_ID initializes regs[BPF_REG_0] without propagating MEM_RCU or
parent_id from the input argument, and the cast block above only runs when
find_reference_state() finds an acquired reference.  Because RCU-protected
and iterator-yielded pointers are tracked via MEM_RCU or parent_id rather
than a reference state, regs[BPF_REG_0] is left as an unconstrained
PTR_TO_BTF_ID.  When the iterator advances or bpf_rcu_read_unlock() is
called, the parent pointer is invalidated while the cast pointer remains
trusted, allowing a use-after-free.

[1] https://sashiko.dev/#/bug/linux-97cfbde4-a454-41bd-bdcf-b13a51d2adaa

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106205523.1765622-1-a.s.protopopov@gmail.com?part=13