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

Yonghong Song <yonghong.song@xxxxxxxxx> · Mon, 12 Aug 2024 11:36:51 -0700

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.








