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

[Yonghong Song <yonghong.song@linux.dev>]

On 8/12/24 11:30 AM, Eduard Zingerman wrote:
> On Mon, 2024-08-12 at 11:26 -0700, Yonghong Song wrote:
>
> [...]
>
>> We could do the following to avoid double comparison: 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; + + 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. WDYT?
>>
> Yonghong, something went wrong with formatting of the above email,
> could you please resend?

Sorry, I copy-paste from 'git diff' result to my email window. Not sure
why it caused the format issue after I sent out. 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;
+
+               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.