Commit 896bcc90 authored by Daniel Borkmann's avatar Daniel Borkmann

Merge branch 'bpf-btf-size-verification-fix'

Andrii Nakryiko says:

====================
BTF size resolution logic isn't always resolving type size correctly, leading
to erroneous map creation failures due to value size mismatch.

This patch set:
1. fixes the issue (patch #1);
2. adds tests for trickier cases (patch #2);
3. and converts few test cases utilizing BTF-defined maps, that previously
   couldn't use typedef'ed arrays due to kernel bug (patch #3).
====================
Acked-by: default avatarYonghong Song <yhs@fb.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
parents af3c24e0 8981e56f
...@@ -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;
} }
......
...@@ -47,11 +47,12 @@ struct { ...@@ -47,11 +47,12 @@ struct {
* issue and avoid complicated C programming massaging. * issue and avoid complicated C programming massaging.
* This is an acceptable workaround since there is one entry here. * This is an acceptable workaround since there is one entry here.
*/ */
typedef __u64 raw_stack_trace_t[2 * MAX_STACK_RAWTP];
struct { struct {
__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY); __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
__uint(max_entries, 1); __uint(max_entries, 1);
__type(key, __u32); __type(key, __u32);
__u64 (*value)[2 * MAX_STACK_RAWTP]; __type(value, raw_stack_trace_t);
} rawdata_map SEC(".maps"); } rawdata_map SEC(".maps");
SEC("tracepoint/raw_syscalls/sys_enter") SEC("tracepoint/raw_syscalls/sys_enter")
......
...@@ -36,8 +36,7 @@ struct { ...@@ -36,8 +36,7 @@ struct {
__uint(type, BPF_MAP_TYPE_ARRAY); __uint(type, BPF_MAP_TYPE_ARRAY);
__uint(max_entries, 128); __uint(max_entries, 128);
__type(key, __u32); __type(key, __u32);
/* there seems to be a bug in kernel not handling typedef properly */ __type(value, stack_trace_t);
struct bpf_stack_build_id (*value)[PERF_MAX_STACK_DEPTH];
} stack_amap SEC(".maps"); } stack_amap SEC(".maps");
/* taken from /sys/kernel/debug/tracing/events/random/urandom_read/format */ /* taken from /sys/kernel/debug/tracing/events/random/urandom_read/format */
......
...@@ -35,7 +35,7 @@ struct { ...@@ -35,7 +35,7 @@ struct {
__uint(type, BPF_MAP_TYPE_ARRAY); __uint(type, BPF_MAP_TYPE_ARRAY);
__uint(max_entries, 16384); __uint(max_entries, 16384);
__type(key, __u32); __type(key, __u32);
__u64 (*value)[PERF_MAX_STACK_DEPTH]; __type(value, stack_trace_t);
} stack_amap SEC(".maps"); } stack_amap SEC(".maps");
/* taken from /sys/kernel/debug/tracing/events/sched/sched_switch/format */ /* taken from /sys/kernel/debug/tracing/events/sched/sched_switch/format */
......
...@@ -3417,6 +3417,94 @@ static struct btf_raw_test raw_tests[] = { ...@@ -3417,6 +3417,94 @@ static struct btf_raw_test raw_tests[] = {
.value_type_id = 1, .value_type_id = 1,
.max_entries = 4, .max_entries = 4,
}, },
/*
* typedef int arr_t[16];
* struct s {
* arr_t *a;
* };
*/
{
.descr = "struct->ptr->typedef->array->int size resolution",
.raw_types = {
BTF_STRUCT_ENC(NAME_TBD, 1, 8), /* [1] */
BTF_MEMBER_ENC(NAME_TBD, 2, 0),
BTF_PTR_ENC(3), /* [2] */
BTF_TYPEDEF_ENC(NAME_TBD, 4), /* [3] */
BTF_TYPE_ARRAY_ENC(5, 5, 16), /* [4] */
BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [5] */
BTF_END_RAW,
},
BTF_STR_SEC("\0s\0a\0arr_t"),
.map_type = BPF_MAP_TYPE_ARRAY,
.map_name = "ptr_mod_chain_size_resolve_map",
.key_size = sizeof(int),
.value_size = sizeof(int) * 16,
.key_type_id = 5 /* int */,
.value_type_id = 3 /* arr_t */,
.max_entries = 4,
},
/*
* typedef int arr_t[16][8][4];
* struct s {
* arr_t *a;
* };
*/
{
.descr = "struct->ptr->typedef->multi-array->int size resolution",
.raw_types = {
BTF_STRUCT_ENC(NAME_TBD, 1, 8), /* [1] */
BTF_MEMBER_ENC(NAME_TBD, 2, 0),
BTF_PTR_ENC(3), /* [2] */
BTF_TYPEDEF_ENC(NAME_TBD, 4), /* [3] */
BTF_TYPE_ARRAY_ENC(5, 7, 16), /* [4] */
BTF_TYPE_ARRAY_ENC(6, 7, 8), /* [5] */
BTF_TYPE_ARRAY_ENC(7, 7, 4), /* [6] */
BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [7] */
BTF_END_RAW,
},
BTF_STR_SEC("\0s\0a\0arr_t"),
.map_type = BPF_MAP_TYPE_ARRAY,
.map_name = "multi_arr_size_resolve_map",
.key_size = sizeof(int),
.value_size = sizeof(int) * 16 * 8 * 4,
.key_type_id = 7 /* int */,
.value_type_id = 3 /* arr_t */,
.max_entries = 4,
},
/*
* typedef int int_t;
* typedef int_t arr3_t[4];
* typedef arr3_t arr2_t[8];
* typedef arr2_t arr1_t[16];
* struct s {
* arr1_t *a;
* };
*/
{
.descr = "typedef/multi-arr mix size resolution",
.raw_types = {
BTF_STRUCT_ENC(NAME_TBD, 1, 8), /* [1] */
BTF_MEMBER_ENC(NAME_TBD, 2, 0),
BTF_PTR_ENC(3), /* [2] */
BTF_TYPEDEF_ENC(NAME_TBD, 4), /* [3] */
BTF_TYPE_ARRAY_ENC(5, 10, 16), /* [4] */
BTF_TYPEDEF_ENC(NAME_TBD, 6), /* [5] */
BTF_TYPE_ARRAY_ENC(7, 10, 8), /* [6] */
BTF_TYPEDEF_ENC(NAME_TBD, 8), /* [7] */
BTF_TYPE_ARRAY_ENC(9, 10, 4), /* [8] */
BTF_TYPEDEF_ENC(NAME_TBD, 10), /* [9] */
BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [10] */
BTF_END_RAW,
},
BTF_STR_SEC("\0s\0a\0arr1_t\0arr2_t\0arr3_t\0int_t"),
.map_type = BPF_MAP_TYPE_ARRAY,
.map_name = "typedef_arra_mix_size_resolve_map",
.key_size = sizeof(int),
.value_size = sizeof(int) * 16 * 8 * 4,
.key_type_id = 10 /* int */,
.value_type_id = 3 /* arr_t */,
.max_entries = 4,
},
}; /* struct btf_raw_test raw_tests[] */ }; /* struct btf_raw_test raw_tests[] */
......
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