Re: [PATCH bpf-next v1 1/2] bpf: Fix array bounds error with may_goto

[Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>]

On Wed, Feb 12, 2025 at 5:53 AM Jiayuan Chen <mrpre@xxxxxxx> wrote:
>
> may_goto uses an additional 8 bytes on the stack, which causes the
> interpreters[] array to go out of bounds when calculating index by
> stack_size.
>
> Reported-by: syzbot+d2a2c639d03ac200a4f1@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/bpf/0000000000000f823606139faa5d@xxxxxxxxxx/
> Fixes: 011832b97b311 ("bpf: Introduce may_goto instruction")
> Signed-off-by: Jiayuan Chen <mrpre@xxxxxxx>
> ---
>  kernel/bpf/core.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index da729cbbaeb9..498b35284f81 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2255,7 +2255,7 @@ static u64 PROG_NAME_ARGS(stack_size)(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5, \
>
>  EVAL6(DEFINE_BPF_PROG_RUN, 32, 64, 96, 128, 160, 192);
>  EVAL6(DEFINE_BPF_PROG_RUN, 224, 256, 288, 320, 352, 384);
> -EVAL4(DEFINE_BPF_PROG_RUN, 416, 448, 480, 512);
> +EVAL5(DEFINE_BPF_PROG_RUN, 416, 448, 480, 512, 544);
>
>  EVAL6(DEFINE_BPF_PROG_RUN_ARGS, 32, 64, 96, 128, 160, 192);
>  EVAL6(DEFINE_BPF_PROG_RUN_ARGS, 224, 256, 288, 320, 352, 384);
> @@ -2267,8 +2267,11 @@ static unsigned int (*interpreters[])(const void *ctx,
>                                       const struct bpf_insn *insn) = {
>  EVAL6(PROG_NAME_LIST, 32, 64, 96, 128, 160, 192)
>  EVAL6(PROG_NAME_LIST, 224, 256, 288, 320, 352, 384)
> -EVAL4(PROG_NAME_LIST, 416, 448, 480, 512)
> +EVAL5(PROG_NAME_LIST, 416, 448, 480, 512, 544)
>  };

That's two extra functions for a rare corner case.
Let's do something like the following instead:
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9971c03adfd5..028de7a6edfc 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -21884,6 +21884,10 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
                        subprogs[cur_subprog].stack_extra = stack_depth_extra;
                        cur_subprog++;
                        stack_depth = subprogs[cur_subprog].stack_depth;
+                       if (stack_depth > MAX_BPF_STACK &&
!prog->jit_requested) {
+                               verbose(...);
+                               return -EINVAL;
+                       }


pw-bot: cr