bpf: verifier: Disambiguate get_constant_map_key() errors

Refactor get_constant_map_key() to disambiguate the constant key
value from potential error values. In the case that the key is
negative, it could be confused for an error.

It's not currently an issue, as the verifier seems to track s32 spills
as u32. So even if the program wrongly uses a negative value for an
arraymap key, the verifier just thinks it's an impossibly high value
which gets correctly discarded.

Refactor anyways to make things cleaner and prevent potential future
issues.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
Link: https://lore.kernel.org/r/dfe144259ae7cfc98aa63e1b388a14869a10632a.1738689872.git.dxu@dxuuu.xyz
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e9176a5ce215ea7206448cb5a6e51291b5705a21..98354d7816789b725caa4032f65d1cda045fd14a 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9149,10 +9149,11 @@ static int check_reg_const_str(struct bpf_verifier_env *env,
        return 0;
 }
 
-/* Returns constant key value if possible, else negative error */
-static s64 get_constant_map_key(struct bpf_verifier_env *env,
+/* Returns constant key value in `value` if possible, else negative error */
+static int get_constant_map_key(struct bpf_verifier_env *env,
                                struct bpf_reg_state *key,
-                               u32 key_size)
+                               u32 key_size,
+                               s64 *value)
 {
        struct bpf_func_state *state = func(env, key);
        struct bpf_reg_state *reg;
@@ -9179,8 +9180,10 @@ static s64 get_constant_map_key(struct bpf_verifier_env *env,
        /* First handle precisely tracked STACK_ZERO */
        for (i = off; i >= 0 && stype[i] == STACK_ZERO; i--)
                zero_size++;
-       if (zero_size >= key_size)
+       if (zero_size >= key_size) {
+               *value = 0;
                return 0;
+       }
 
        /* Check that stack contains a scalar spill of expected size */
        if (!is_spilled_scalar_reg(&state->stack[spi]))
@@ -9203,7 +9206,8 @@ static s64 get_constant_map_key(struct bpf_verifier_env *env,
        if (err < 0)
                return err;
 
-       return reg->var_off.value;
+       *value = reg->var_off.value;
+       return 0;
 }
 
 static bool can_elide_value_nullness(enum bpf_map_type type);
@@ -9357,9 +9361,14 @@ skip_type_check:
                if (err)
                        return err;
                if (can_elide_value_nullness(meta->map_ptr->map_type)) {
-                       meta->const_map_key = get_constant_map_key(env, reg, key_size);
-                       if (meta->const_map_key < 0 && meta->const_map_key != -EOPNOTSUPP)
-                               return meta->const_map_key;
+                       err = get_constant_map_key(env, reg, key_size, &meta->const_map_key);
+                       if (err < 0) {
+                               meta->const_map_key = -1;
+                               if (err == -EOPNOTSUPP)
+                                       err = 0;
+                               else
+                                       return err;
+                       }
                }
                break;
        case ARG_PTR_TO_MAP_VALUE: