Re: [PATCH bpf v4 2/3] bpf: Add validation for bpf_set_retval argument

[bot+bpf-ci]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 7fb88e1cd7c4..1bf58685b655 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -9952,6 +9952,7 @@ static int do_refine_retval_range(struct bpf_verifier_env *env,
>  				  int func_id,
>  				  struct bpf_call_arg_meta *meta)
>  {
> +	struct bpf_retval_range range;
>  	struct bpf_reg_state *ret_reg = &regs[BPF_REG_0];
>
>  	if (ret_type != RET_INTEGER)
> @@ -9980,6 +9981,29 @@ static int do_refine_retval_range(struct bpf_verifier_env *env,
>  		ret_reg->s32_min_value = 0;
>  		reg_bounds_sync(ret_reg);
>  		break;
> +	case BPF_FUNC_get_retval:
> +		/*
> +		 * bpf_get_reval may see arbitrary value passed by bpf_prog_run_array_cg for
> +		 * CGROUP_GETSOCKOPT type.
> +		 */
> +		if (env->prog->type == BPF_PROG_TYPE_CGROUP_SOCKOPT &&
> +		    env->prog->expected_attach_type == BPF_CGROUP_GETSOCKOPT)
> +			break;
> +
> +		if (env->prog->type == BPF_PROG_TYPE_LSM &&
> +		    env->prog->expected_attach_type == BPF_LSM_CGROUP) {
> +			if (!env->prog->aux->attach_func_proto->type)
> +				break;
> +			bpf_lsm_get_retval_range(env->prog, &range);
> +		} else {
> +			range.minval = -MAX_ERRNO;
> +			range.maxval = 0;
> +		}
> +
> +		reg_set_srange64(ret_reg, range.minval, range.maxval);
> +		reg_set_srange32(ret_reg, range.minval, range.maxval);
                    ^^^^^^^^^^^^^^^

Are reg_set_srange64() and reg_set_srange32() defined in this kernel? A
search across the entire tree finds only these two call sites and no
definition. The struct bpf_retval_range has minval/maxval fields but no
associated reg_set_srange* helper exists in this codebase.

Looking at the existing pattern in this same function (lines 9969-9970),
the code directly assigns fields:

    ret_reg->smin_value = -MAX_ERRNO;
    ret_reg->s32_min_value = -MAX_ERRNO;

The reg_set_srange* helpers appear to belong to a different verifier
variant using reg->r64 = cnum64_from_srange(...) that isn't present here.

Should this use the same pattern as the BPF_FUNC_get_stack case above it:

    ret_reg->smin_value = range.minval;
    ret_reg->smax_value = range.maxval;
    ret_reg->s32_min_value = range.minval;
    ret_reg->s32_max_value = range.maxval;
    reg_bounds_sync(ret_reg);

This would match the existing codebase and avoid undefined function
references that will fail to compile.

> +		reg_bounds_sync(ret_reg);
> +		break;
>  	}
>
>  	return reg_bounds_sanity_check(env, ret_reg, "retval");

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26955107708