Re: [PATCH bpf 1/2] bpf: Fix a kernel verifier crash in stacksafe()

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2024-08-12 at 11:36 -0700, Yonghong Song wrote:

[...]

> Sorry, I copy-paste from 'git diff' result to my email window. Not sure
> why it caused the format issue after I sent out.

Sure, no problem

> Anyway, the following is the patch I suggested:
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index df3be12096cf..1906798f1a3d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17338,10 +17338,13 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
>           */
>          for (i = 0; i < old->allocated_stack; i++) {
>                  struct bpf_reg_state *old_reg, *cur_reg;
> +               bool cur_exceed_bound;
>   
>                  spi = i / BPF_REG_SIZE;
>   
> -               if (exact != NOT_EXACT &&
> +               cur_exceed_bound = i >= cur->allocated_stack;

idk, I think C compiler would do this anyways,
to me the code is fine both with and without this additional variable.

> +
> +               if (exact != NOT_EXACT && !cur_exceed_bound &&
>                      old->stack[spi].slot_type[i % BPF_REG_SIZE] !=
>                      cur->stack[spi].slot_type[i % BPF_REG_SIZE])
>                          return false;
> @@ -17363,7 +17366,7 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
>                  /* explored stack has more populated slots than current stack
>                   * and these slots were used
>                   */
> -               if (i >= cur->allocated_stack)
> +               if (cur_exceed_bound)
>                          return false;
>   
>                  /* 64-bit scalar spill vs all slots MISC and vice versa.
>