[PATCH bpf] bpf: Check the helper function is valid in get_helper_proto

[PATCH bpf] bpf: Check the helper function is valid in get_helper_proto

[Jiri Olsa]

From: Jiri Olsa <olsajiri@gmail.com>

syzbot reported an verifier bug [1] where the helper func pointer
could be NULL due to disabled config option.

As Alexei suggested we could check on that in get_helper_proto
directly. Excluding tail_call helper from the check, because it
is NULL by design and valid in all configs.

[1] https://lore.kernel.org/bpf/68904050.050a0220.7f033.0001.GAE@google.com/
Reported-by: syzbot+a9ed3d9132939852d0df@syzkaller.appspotmail.com
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/verifier.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c4f69a9e9af6..5e38489656e2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11344,6 +11344,13 @@ static bool can_elide_value_nullness(enum bpf_map_type type)
 	}
 }
 
+static bool is_valid_proto(const struct bpf_func_proto *fn)
+{
+	if (fn == &bpf_tail_call_proto)
+		return true;
+	return fn && fn->func;
+}
+
 static int get_helper_proto(struct bpf_verifier_env *env, int func_id,
 			    const struct bpf_func_proto **ptr)
 {
@@ -11354,7 +11361,7 @@ static int get_helper_proto(struct bpf_verifier_env *env, int func_id,
 		return -EINVAL;
 
 	*ptr = env->ops->get_func_proto(func_id, env->prog);
-	return *ptr ? 0 : -EINVAL;
+	return is_valid_proto(*ptr) ? 0 : -EINVAL;
 }
 
 static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
-- 
2.50.1

Re: [PATCH bpf] bpf: Check the helper function is valid in get_helper_proto

[Andrii Nakryiko]

On Tue, Aug 12, 2025 at 3:12 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> From: Jiri Olsa <olsajiri@gmail.com>
>
> syzbot reported an verifier bug [1] where the helper func pointer
> could be NULL due to disabled config option.
>
> As Alexei suggested we could check on that in get_helper_proto
> directly. Excluding tail_call helper from the check, because it
> is NULL by design and valid in all configs.
>
> [1] https://lore.kernel.org/bpf/68904050.050a0220.7f033.0001.GAE@google.com/
> Reported-by: syzbot+a9ed3d9132939852d0df@syzkaller.appspotmail.com
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  kernel/bpf/verifier.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index c4f69a9e9af6..5e38489656e2 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11344,6 +11344,13 @@ static bool can_elide_value_nullness(enum bpf_map_type type)
>         }
>  }
>
> +static bool is_valid_proto(const struct bpf_func_proto *fn)
> +{
> +       if (fn == &bpf_tail_call_proto)
> +               return true;

ugh... what if we set bpf_tail_call_proto's .func to (void *)0xDEADBAD
or some such and avoid this special casing?

> +       return fn && fn->func;
> +}
> +
>  static int get_helper_proto(struct bpf_verifier_env *env, int func_id,
>                             const struct bpf_func_proto **ptr)
>  {
> @@ -11354,7 +11361,7 @@ static int get_helper_proto(struct bpf_verifier_env *env, int func_id,
>                 return -EINVAL;
>
>         *ptr = env->ops->get_func_proto(func_id, env->prog);
> -       return *ptr ? 0 : -EINVAL;

so we explicitly do not want WARN/BUG/verifier_bug() if
!is_valid_proto(), is that right?

> +       return is_valid_proto(*ptr) ? 0 : -EINVAL;
>  }
>
>  static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> --
> 2.50.1
>

Re: [PATCH bpf] bpf: Check the helper function is valid in get_helper_proto

[Jiri Olsa]

On Tue, Aug 12, 2025 at 03:32:40PM -0700, Andrii Nakryiko wrote:
> On Tue, Aug 12, 2025 at 3:12 PM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > From: Jiri Olsa <olsajiri@gmail.com>
> >
> > syzbot reported an verifier bug [1] where the helper func pointer
> > could be NULL due to disabled config option.
> >
> > As Alexei suggested we could check on that in get_helper_proto
> > directly. Excluding tail_call helper from the check, because it
> > is NULL by design and valid in all configs.
> >
> > [1] https://lore.kernel.org/bpf/68904050.050a0220.7f033.0001.GAE@google.com/
> > Reported-by: syzbot+a9ed3d9132939852d0df@syzkaller.appspotmail.com
> > Suggested-by: Alexei Starovoitov <ast@kernel.org>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  kernel/bpf/verifier.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index c4f69a9e9af6..5e38489656e2 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -11344,6 +11344,13 @@ static bool can_elide_value_nullness(enum bpf_map_type type)
> >         }
> >  }
> >
> > +static bool is_valid_proto(const struct bpf_func_proto *fn)
> > +{
> > +       if (fn == &bpf_tail_call_proto)
> > +               return true;
> 
> ugh... what if we set bpf_tail_call_proto's .func to (void *)0xDEADBAD
> or some such and avoid this special casing?

right, that's an option, will change

> 
> > +       return fn && fn->func;
> > +}
> > +
> >  static int get_helper_proto(struct bpf_verifier_env *env, int func_id,
> >                             const struct bpf_func_proto **ptr)
> >  {
> > @@ -11354,7 +11361,7 @@ static int get_helper_proto(struct bpf_verifier_env *env, int func_id,
> >                 return -EINVAL;
> >
> >         *ptr = env->ops->get_func_proto(func_id, env->prog);
> > -       return *ptr ? 0 : -EINVAL;
> 
> so we explicitly do not want WARN/BUG/verifier_bug() if
> !is_valid_proto(), is that right?

yes, I don't think it's verifier bug if option is missing, with this change
we will fail earlier in check_helper_call->get_helper_proto

jirka

> 
> > +       return is_valid_proto(*ptr) ? 0 : -EINVAL;
> >  }
> >
> >  static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> > --
> > 2.50.1
> >