On Sat, Jun 29, 2024 at 2:48 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> Extract the part of check_helper_call() as a utility function allowing
> to query 'struct bpf_func_proto' for a specific helper function id.
>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> kernel/bpf/verifier.c | 37 ++++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d3927d819465..8dd3385cf925 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10261,6 +10261,24 @@ static void update_loop_inline_state(struct bpf_verifier_env *env, u32 subprogno
> state->callback_subprogno == subprogno);
> }
>
> +static int get_helper_proto(struct bpf_verifier_env *env, int func_id,
> + const struct bpf_func_proto **ptr)
> +{
> + const struct bpf_func_proto *result = NULL;
> +
> + if (func_id < 0 || func_id >= __BPF_FUNC_MAX_ID)
> + return -ERANGE;
> +
> + if (env->ops->get_func_proto)
> + result = env->ops->get_func_proto(func_id, env->prog);
> +
> + if (!result)
> + return -EINVAL;
> +
> + *ptr = result;
> + return 0;
> +}
> +
> static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> int *insn_idx_p)
> {
> @@ -10277,17 +10295,14 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>
> /* find function prototype */
> func_id = insn->imm;
> - if (func_id < 0 || func_id >= __BPF_FUNC_MAX_ID) {
> - verbose(env, "invalid func %s#%d\n", func_id_name(func_id),
> - func_id);
> - return -EINVAL;
> - }
> -
> - if (env->ops->get_func_proto)
> - fn = env->ops->get_func_proto(func_id, env->prog);
> - if (!fn) {
> - verbose(env, "program of this type cannot use helper %s#%d\n",
> - func_id_name(func_id), func_id);
> + err = get_helper_proto(env, insn->imm, &fn);
> + if (err) {
> + if (err == -ERANGE)
> + verbose(env, "invalid func %s#%d\n", func_id_name(func_id),
> + func_id);
> + else
> + verbose(env, "program of this type cannot use helper %s#%d\n",
> + func_id_name(func_id), func_id);
> return -EINVAL;
> }
subjective, but this might be cleaner and will keep first verbose() on
a single line:
err = get_helper_proto(...);
if (err == -ERANGE) {
verbose(...);
return -EINVAL;
} else if (err) {
verbose(...);
return err;
}
>
> --
> 2.45.2
>
