As of recently, Clang is able to attach type tag records to modifier BTF records. This is useful for using typedefs that encompass a base type and a type tag, e.g.: typedef struct rbtree __arena rbtree_t; Modify btf_check_type_tags() so that it allows this sequence of records. The function now only checks for record loops in BTF modifier record chains. Rename to btf_check_modifier_chain_length to reflect this. Also expand the BTF modifier traversal code to take into account that type record can be interleaved with other modifier records. In effect this means traversing all modifiers to collect the type tags. Also modify existing selftests to now accept modifier records (const, typedef) that point to type tag records. Cc: Yonghong Song Cc: Vineet Gupta Signed-off-by: Emil Tsalapatis --- kernel/bpf/btf.c | 145 ++++++++++++------- tools/testing/selftests/bpf/prog_tests/btf.c | 12 +- 2 files changed, 94 insertions(+), 63 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index ef4402274786..b7954f42b396 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -3475,9 +3475,8 @@ static int btf_find_struct(const struct btf *btf, const struct btf_type *t, static int btf_find_kptr(const struct btf *btf, const struct btf_type *t, u32 off, int sz, struct btf_field_info *info, u32 field_mask) { - enum btf_field_type type; + enum btf_field_type type = 0; const char *tag_value; - bool is_type_tag; u32 res_id; /* Permit modifiers on the pointer itself */ @@ -3486,30 +3485,37 @@ static int btf_find_kptr(const struct btf *btf, const struct btf_type *t, /* For PTR, sz is always == 8 */ if (!btf_type_is_ptr(t)) return BTF_FIELD_IGNORE; - t = btf_type_by_id(btf, t->type); - is_type_tag = btf_type_is_type_tag(t) && !btf_type_kflag(t); - if (!is_type_tag) - return BTF_FIELD_IGNORE; - /* Reject extra tags */ - if (btf_type_is_type_tag(btf_type_by_id(btf, t->type))) - return -EINVAL; - tag_value = __btf_name_by_offset(btf, t->name_off); - if (!strcmp("kptr_untrusted", tag_value)) - type = BPF_KPTR_UNREF; - else if (!strcmp("kptr", tag_value)) - type = BPF_KPTR_REF; - else if (!strcmp("percpu_kptr", tag_value)) - type = BPF_KPTR_PERCPU; - else if (!strcmp("uptr", tag_value)) - type = BPF_UPTR; - else - return -EINVAL; + + res_id = t->type; + t = btf_type_by_id(btf, res_id); + while (btf_type_is_modifier(t)) { + if (!btf_type_is_type_tag(t) || btf_type_kflag(t)) + goto skip_modifier; + + /* Reject extra tags */ + if (type) + return -EINVAL; + + tag_value = __btf_name_by_offset(btf, t->name_off); + if (!strcmp("kptr_untrusted", tag_value)) + type = BPF_KPTR_UNREF; + else if (!strcmp("kptr", tag_value)) + type = BPF_KPTR_REF; + else if (!strcmp("percpu_kptr", tag_value)) + type = BPF_KPTR_PERCPU; + else if (!strcmp("uptr", tag_value)) + type = BPF_UPTR; + else + return -EINVAL; + +skip_modifier: + res_id = t->type; + t = btf_type_by_id(btf, res_id); + } if (!(type & field_mask)) return BTF_FIELD_IGNORE; - /* Get the base type */ - t = btf_type_skip_modifiers(btf, t->type, &res_id); /* Only pointer to struct is allowed */ if (!__btf_type_is_struct(t)) return -EINVAL; @@ -5860,11 +5866,10 @@ struct btf_struct_meta *btf_find_struct_meta(const struct btf *btf, u32 btf_id) return bsearch(&btf_id, tab->types, tab->cnt, sizeof(tab->types[0]), btf_id_cmp_func); } -static int btf_check_type_tags(struct btf_verifier_env *env, +static int btf_check_modifier_chain_length(struct btf_verifier_env *env, struct btf *btf, int start_id) { int i, n, good_id = start_id - 1; - bool in_tags; n = btf_nr_types(btf); for (i = start_id; i < n; i++) { @@ -5880,20 +5885,12 @@ static int btf_check_type_tags(struct btf_verifier_env *env, cond_resched(); - in_tags = btf_type_is_type_tag(t); while (btf_type_is_modifier(t)) { if (!chain_limit--) { btf_verifier_log(env, "Max chain length or cycle detected"); return -ELOOP; } - if (btf_type_is_type_tag(t)) { - if (!in_tags) { - btf_verifier_log(env, "Type tags don't precede modifiers"); - return -EINVAL; - } - } else if (in_tags) { - in_tags = false; - } + if (cur_id <= good_id) break; /* Move to next type */ @@ -5971,7 +5968,7 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, if (err) goto errout; - err = btf_check_type_tags(env, btf, 1); + err = btf_check_modifier_chain_length(env, btf, 1); if (err) goto errout; @@ -6379,7 +6376,7 @@ static struct btf *btf_parse_base(struct btf_verifier_env *env, const char *name if (err) goto errout; - err = btf_check_type_tags(env, btf, 1); + err = btf_check_modifier_chain_length(env, btf, 1); if (err) goto errout; @@ -6505,7 +6502,7 @@ static struct btf *btf_parse_module(const char *module_name, const void *data, if (err) goto errout; - err = btf_check_type_tags(env, btf, btf_nr_types(base_btf)); + err = btf_check_modifier_chain_length(env, btf, btf_nr_types(base_btf)); if (err) goto errout; @@ -6818,7 +6815,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, struct bpf_verifier_log *log = info->log; const struct btf_param *args; bool ptr_err_raw_tp = false; - const char *tag_value; + const char *tag_value = NULL; u32 nr_args, arg; int i, ret; @@ -7024,19 +7021,43 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, info->btf_id = t->type; t = btf_type_by_id(btf, t->type); - if (btf_type_is_type_tag(t) && !btf_type_kflag(t)) { - tag_value = __btf_name_by_offset(btf, t->name_off); - if (strcmp(tag_value, "user") == 0) + while (btf_type_is_modifier(t)) { + const char *new_tag; + + if (!btf_type_is_type_tag(t) || btf_type_kflag(t)) + goto skip_modifier; + + new_tag = __btf_name_by_offset(btf, t->name_off); + if (strcmp(new_tag, "user") == 0) { info->reg_type |= MEM_USER; - if (strcmp(tag_value, "percpu") == 0) + } else if (strcmp(new_tag, "percpu") == 0) { info->reg_type |= MEM_PERCPU; - } + } else { + /* + * If we fail to match on the tag value just ignore it. + * Tracing programs have the same BTF as the function + * being traced, possibly including type tags BPF is not + * aware of. We cannot possibly account for them so ignore + * them. + */ + goto skip_modifier; + } - /* skip modifiers */ - while (btf_type_is_modifier(t)) { + /* Only a single tag value supported. */ + if (tag_value) { + bpf_log(log, + "func '%s' arg%d type %s has multiple type tags\n", + tname, arg, btf_type_str(t)); + return false; + } + + tag_value = new_tag; + +skip_modifier: info->btf_id = t->type; t = btf_type_by_id(btf, t->type); } + if (!btf_type_is_struct(t)) { bpf_log(log, "func '%s' arg%d type %s is not a struct\n", @@ -7075,7 +7096,7 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf, u32 i, moff, mtrue_end, msize = 0, total_nelems = 0; const struct btf_type *mtype, *elem_type = NULL; const struct btf_member *member; - const char *tname, *mname, *tag_value; + const char *tname, *mname; u32 vlen, elem_id, mid; again: @@ -7271,8 +7292,10 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf, } if (btf_type_is_ptr(mtype)) { - const struct btf_type *stype, *t; enum bpf_type_flag tmp_flag = 0; + const struct btf_type *stype; + const char *tag_value = NULL; + bool tag_found = false; u32 id; if (msize != size || off != moff) { @@ -7282,22 +7305,38 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf, return -EACCES; } - /* check type tag */ - t = btf_type_by_id(btf, mtype->type); - if (btf_type_is_type_tag(t) && !btf_type_kflag(t)) { - tag_value = __btf_name_by_offset(btf, t->name_off); + id = mtype->type; + stype = btf_type_by_id(btf, id); + while (btf_type_is_modifier(stype)) { + if (!btf_type_is_type_tag(stype) || btf_type_kflag(stype)) + goto skip_modifier; + + tag_value = __btf_name_by_offset(btf, stype->name_off); /* check __user tag */ if (strcmp(tag_value, "user") == 0) tmp_flag = MEM_USER; /* check __percpu tag */ - if (strcmp(tag_value, "percpu") == 0) + else if (strcmp(tag_value, "percpu") == 0) tmp_flag = MEM_PERCPU; /* check __rcu tag */ - if (strcmp(tag_value, "rcu") == 0) + else if (strcmp(tag_value, "rcu") == 0) tmp_flag = MEM_RCU; + else + goto skip_modifier; + + if (tag_found) { + bpf_log(log, + "type '%s' has multiple type tags\n", + btf_type_str(stype)); + return -EINVAL; + } + tag_found = true; + +skip_modifier: + id = stype->type; + stype = btf_type_by_id(btf, id); } - stype = btf_type_skip_modifiers(btf, mtype->type, &id); if (btf_type_is_struct(stype)) { *next_btf_id = id; *flag |= tmp_flag; diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c index a9de328a8697..df785a00f0b4 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf.c +++ b/tools/testing/selftests/bpf/prog_tests/btf.c @@ -4121,8 +4121,6 @@ static struct btf_raw_test raw_tests[] = { .key_type_id = 1, .value_type_id = 1, .max_entries = 1, - .btf_load_err = true, - .err_str = "Type tags don't precede modifiers", }, { .descr = "type_tag test #3, type tag order", @@ -4141,8 +4139,6 @@ static struct btf_raw_test raw_tests[] = { .key_type_id = 1, .value_type_id = 1, .max_entries = 1, - .btf_load_err = true, - .err_str = "Type tags don't precede modifiers", }, { .descr = "type_tag test #4, type tag order", @@ -4161,8 +4157,6 @@ static struct btf_raw_test raw_tests[] = { .key_type_id = 1, .value_type_id = 1, .max_entries = 1, - .btf_load_err = true, - .err_str = "Type tags don't precede modifiers", }, { .descr = "type_tag test #5, type tag order", @@ -4198,11 +4192,9 @@ static struct btf_raw_test raw_tests[] = { .map_name = "tag_type_check_btf", .key_size = sizeof(int), .value_size = 4, - .key_type_id = 1, - .value_type_id = 1, + .key_type_id = 4, + .value_type_id = 4, .max_entries = 1, - .btf_load_err = true, - .err_str = "Type tags don't precede modifiers", }, { .descr = "type_tag test #7, tag with kflag", -- 2.54.0