Commit 1acc5d5c authored by Andrii Nakryiko's avatar Andrii Nakryiko Committed by Daniel Borkmann

bpf: fix BTF verifier size resolution logic

BTF verifier has a size resolution bug which in some circumstances leads to
invalid size resolution for, e.g., TYPEDEF modifier.  This happens if we have
[1] PTR -> [2] TYPEDEF -> [3] ARRAY, in which case due to being in pointer
context ARRAY size won't be resolved (because for pointer it doesn't matter, so
it's a sink in pointer context), but it will be permanently remembered as zero
for TYPEDEF and TYPEDEF will be marked as RESOLVED. Eventually ARRAY size will
be resolved correctly, but TYPEDEF resolved_size won't be updated anymore.
This, subsequently, will lead to erroneous map creation failure, if that
TYPEDEF is specified as either key or value, as key_size/value_size won't
correspond to resolved size of TYPEDEF (kernel will believe it's zero).

Note, that if BTF was ordered as [1] ARRAY <- [2] TYPEDEF <- [3] PTR, this
won't be a problem, as by the time we get to TYPEDEF, ARRAY's size is already
calculated and stored.

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");

The fix consists on not relying on modifier's resolved_size and instead using
modifier's resolved_id (type ID for "concrete" type to which modifier
eventually resolves) and doing size determination for that resolved type. This
allow to preserve existing "early DFS termination" logic for PTR or
STRUCT_OR_ARRAY contexts, but still do correct size determination for modifier
types.

Fixes: eb3f595d ("bpf: btf: Validate type reference")
Cc: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: default avatarAndrii Nakryiko <andriin@fb.com>
Acked-by: default avatarMartin KaFai Lau <kafai@fb.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
parent af3c24e0
...@@ -1073,11 +1073,18 @@ const struct btf_type *btf_type_id_size(const struct btf *btf, ...@@ -1073,11 +1073,18 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
!btf_type_is_var(size_type))) !btf_type_is_var(size_type)))
return NULL; return NULL;
size = btf->resolved_sizes[size_type_id];
size_type_id = btf->resolved_ids[size_type_id]; size_type_id = btf->resolved_ids[size_type_id];
size_type = btf_type_by_id(btf, size_type_id); size_type = btf_type_by_id(btf, size_type_id);
if (btf_type_nosize_or_null(size_type)) if (btf_type_nosize_or_null(size_type))
return NULL; return NULL;
else if (btf_type_has_size(size_type))
size = size_type->size;
else if (btf_type_is_array(size_type))
size = btf->resolved_sizes[size_type_id];
else if (btf_type_is_ptr(size_type))
size = sizeof(void *);
else
return NULL;
} }
*type_id = size_type_id; *type_id = size_type_id;
...@@ -1602,7 +1609,6 @@ static int btf_modifier_resolve(struct btf_verifier_env *env, ...@@ -1602,7 +1609,6 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
const struct btf_type *next_type; const struct btf_type *next_type;
u32 next_type_id = t->type; u32 next_type_id = t->type;
struct btf *btf = env->btf; struct btf *btf = env->btf;
u32 next_type_size = 0;
next_type = btf_type_by_id(btf, next_type_id); next_type = btf_type_by_id(btf, next_type_id);
if (!next_type || btf_type_is_resolve_source_only(next_type)) { if (!next_type || btf_type_is_resolve_source_only(next_type)) {
...@@ -1620,7 +1626,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env, ...@@ -1620,7 +1626,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
* save us a few type-following when we use it later (e.g. in * save us a few type-following when we use it later (e.g. in
* pretty print). * pretty print).
*/ */
if (!btf_type_id_size(btf, &next_type_id, &next_type_size)) { if (!btf_type_id_size(btf, &next_type_id, NULL)) {
if (env_type_is_resolved(env, next_type_id)) if (env_type_is_resolved(env, next_type_id))
next_type = btf_type_id_resolve(btf, &next_type_id); next_type = btf_type_id_resolve(btf, &next_type_id);
...@@ -1633,7 +1639,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env, ...@@ -1633,7 +1639,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
} }
} }
env_stack_pop_resolved(env, next_type_id, next_type_size); env_stack_pop_resolved(env, next_type_id, 0);
return 0; return 0;
} }
...@@ -1645,7 +1651,6 @@ static int btf_var_resolve(struct btf_verifier_env *env, ...@@ -1645,7 +1651,6 @@ static int btf_var_resolve(struct btf_verifier_env *env,
const struct btf_type *t = v->t; const struct btf_type *t = v->t;
u32 next_type_id = t->type; u32 next_type_id = t->type;
struct btf *btf = env->btf; struct btf *btf = env->btf;
u32 next_type_size;
next_type = btf_type_by_id(btf, next_type_id); next_type = btf_type_by_id(btf, next_type_id);
if (!next_type || btf_type_is_resolve_source_only(next_type)) { if (!next_type || btf_type_is_resolve_source_only(next_type)) {
...@@ -1675,12 +1680,12 @@ static int btf_var_resolve(struct btf_verifier_env *env, ...@@ -1675,12 +1680,12 @@ static int btf_var_resolve(struct btf_verifier_env *env,
* forward types or similar that would resolve to size of * forward types or similar that would resolve to size of
* zero is allowed. * zero is allowed.
*/ */
if (!btf_type_id_size(btf, &next_type_id, &next_type_size)) { if (!btf_type_id_size(btf, &next_type_id, NULL)) {
btf_verifier_log_type(env, v->t, "Invalid type_id"); btf_verifier_log_type(env, v->t, "Invalid type_id");
return -EINVAL; return -EINVAL;
} }
env_stack_pop_resolved(env, next_type_id, next_type_size); env_stack_pop_resolved(env, next_type_id, 0);
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment