BTF verifier has Different logic depending on whether we are following a PTR or STRUCT/ARRAY (or something else). This is an optimization to stop early in DFS traversal while resolving BTF types. But it also results in a size resolution bug, when there is a chain, e.g., of PTR -> TYPEDEF -> ARRAY, in which case due to being in pointer context ARRAY size won't be resolved, as it is considered to be a sink for pointer, leading to TYPEDEF being in RESOLVED state with zero size, which is completely wrong. Optimization is doubtful, though, as btf_check_all_types() will iterate over all BTF types anyways, so the only saving is a potentially slightly shorter stack. But correctness is more important that tiny savings. This bug manifests itself in rejecting BTF-defined maps that use array typedef as a value type: typedef int array_t[16]; struct { __uint(type, BPF_MAP_TYPE_ARRAY); __type(value, array_t); /* i.e., array_t *value; */ } test_map SEC(".maps"); Fixes: eb3f595dab40 ("bpf: btf: Validate type reference") Cc: Martin KaFai Lau <kafai@xxxxxx> Signed-off-by: Andrii Nakryiko <andriin@xxxxxx> --- kernel/bpf/btf.c | 42 +++--------------------------------------- 1 file changed, 3 insertions(+), 39 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index cad09858a5f2..c68c7e73b0d1 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -231,14 +231,6 @@ enum visit_state { RESOLVED, }; -enum resolve_mode { - RESOLVE_TBD, /* To Be Determined */ - RESOLVE_PTR, /* Resolving for Pointer */ - RESOLVE_STRUCT_OR_ARRAY, /* Resolving for struct/union - * or array - */ -}; - #define MAX_RESOLVE_DEPTH 32 struct btf_sec_info { @@ -254,7 +246,6 @@ struct btf_verifier_env { u32 log_type_id; u32 top_stack; enum verifier_phase phase; - enum resolve_mode resolve_mode; }; static const char * const btf_kind_str[NR_BTF_KINDS] = { @@ -964,26 +955,7 @@ static void btf_verifier_env_free(struct btf_verifier_env *env) static bool env_type_is_resolve_sink(const struct btf_verifier_env *env, const struct btf_type *next_type) { - switch (env->resolve_mode) { - case RESOLVE_TBD: - /* int, enum or void is a sink */ - return !btf_type_needs_resolve(next_type); - case RESOLVE_PTR: - /* int, enum, void, struct, array, func or func_proto is a sink - * for ptr - */ - return !btf_type_is_modifier(next_type) && - !btf_type_is_ptr(next_type); - case RESOLVE_STRUCT_OR_ARRAY: - /* int, enum, void, ptr, func or func_proto is a sink - * for struct and array - */ - return !btf_type_is_modifier(next_type) && - !btf_type_is_array(next_type) && - !btf_type_is_struct(next_type); - default: - BUG(); - } + return !btf_type_needs_resolve(next_type); } static bool env_type_is_resolved(const struct btf_verifier_env *env, @@ -1010,13 +982,6 @@ static int env_stack_push(struct btf_verifier_env *env, v->type_id = type_id; v->next_member = 0; - if (env->resolve_mode == RESOLVE_TBD) { - if (btf_type_is_ptr(t)) - env->resolve_mode = RESOLVE_PTR; - else if (btf_type_is_struct(t) || btf_type_is_array(t)) - env->resolve_mode = RESOLVE_STRUCT_OR_ARRAY; - } - return 0; } @@ -1038,7 +1003,7 @@ static void env_stack_pop_resolved(struct btf_verifier_env *env, env->visit_states[type_id] = RESOLVED; } -static const struct resolve_vertex *env_stack_peak(struct btf_verifier_env *env) +static const struct resolve_vertex *env_stack_peek(struct btf_verifier_env *env) { return env->top_stack ? &env->stack[env->top_stack - 1] : NULL; } @@ -3030,9 +2995,8 @@ static int btf_resolve(struct btf_verifier_env *env, const struct resolve_vertex *v; int err = 0; - env->resolve_mode = RESOLVE_TBD; env_stack_push(env, t, type_id); - while (!err && (v = env_stack_peak(env))) { + while (!err && (v = env_stack_peek(env))) { env->log_type_id = v->type_id; err = btf_type_ops(v->t)->resolve(env, v); } -- 2.17.1