| Message ID | 20200819224030.1615203-4-haoluo@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bpf: BTF support for ksyms | expand |
On 8/19/20 3:40 PM, Hao Luo wrote: > For a ksym to be safely dereferenced and accessed, its type defined in > bpf program should basically match its type defined in kernel. Implement > a help function for a quick matching, which is used by libbpf when > resolving the kernel btf_id of a ksym. > > Signed-off-by: Hao Luo <haoluo@google.com> > --- > tools/lib/bpf/btf.c | 171 ++++++++++++++++++++++++++++++++++++++++++++ > tools/lib/bpf/btf.h | 2 + > 2 files changed, 173 insertions(+) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index a3d259e614b0..2ff31f244d7a 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -1005,6 +1005,177 @@ int btf__get_map_kv_tids(const struct btf *btf, const char *map_name, > return 0; > } > > +/* > + * Basic type check for ksym support. Only checks type kind and resolved size. > + */ > +static inline > +bool btf_ksym_equal_type(const struct btf *ba, __u32 type_a, > + const struct btf *bb, __u32 type_b) "ba" and "bb" is not descriptive. Maybe "btf_a" or "btf_b"? or even "btf1" or "btf2" since the number does not carry extra meaning compared to letters. The same for below, may be t1, t2? > +{ > + const struct btf_type *ta, *tb; > + > + ta = btf__type_by_id(ba, type_a); > + tb = btf__type_by_id(bb, type_b); > + > + /* compare type kind */ > + if (btf_kind(ta) != btf_kind(tb)) > + return false; > + > + /* compare resolved type size */ > + return btf__resolve_size(ba, type_a) == btf__resolve_size(bb, type_b); > +} > + > +/* > + * Match a ksym's type defined in bpf programs against its type encoded in > + * kernel btf. > + */ > +bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, > + const struct btf *bb, __u32 id_b) > +{ > + const struct btf_type *ta = btf__type_by_id(ba, id_a); > + const struct btf_type *tb = btf__type_by_id(bb, id_b); > + int i; > + > + /* compare type kind */ > + if (btf_kind(ta) != btf_kind(tb)) { > + pr_warn("%s:mismatched type kind (%d v.s. %d).\n", > + __func__, btf_kind(ta), btf_kind(tb)); > + return false; > + } > + > + switch (btf_kind(ta)) { > + case BTF_KIND_INT: { /* compare size and encoding */ > + __u32 ea, eb; > + > + if (ta->size != tb->size) { > + pr_warn("%s:INT size mismatch, (%u v.s. %u)\n", > + __func__, ta->size, tb->size); > + return false; > + } > + ea = *(__u32 *)(ta + 1); > + eb = *(__u32 *)(tb + 1); > + if (ea != eb) { > + pr_warn("%s:INT encoding mismatch (%u v.s. %u)\n", > + __func__, ea, eb); > + return false; > + } > + break; > + } > + case BTF_KIND_ARRAY: { /* compare type and number of elements */ > + const struct btf_array *ea, *eb; > + > + ea = btf_array(ta); > + eb = btf_array(tb); > + if (!btf_ksym_equal_type(ba, ea->type, bb, eb->type)) { > + pr_warn("%s:ARRAY elem type mismatch.\n", __func__); > + return false; > + } > + if (ea->nelems != eb->nelems) { > + pr_warn("%s:ARRAY nelems mismatch (%d v.s. %d)\n", > + __func__, ea->nelems, eb->nelems); > + return false; > + } > + break; > + } > + case BTF_KIND_STRUCT: > + case BTF_KIND_UNION: { /* compare size, vlen and member offset, name */ > + const struct btf_member *ma, *mb; > + > + if (ta->size != tb->size) { > + pr_warn("%s:STRUCT size mismatch, (%u v.s. %u)\n", > + __func__, ta->size, tb->size); > + return false; > + } > + if (btf_vlen(ta) != btf_vlen(tb)) { > + pr_warn("%s:STRUCT vlen mismatch, (%u v.s. %u)\n", > + __func__, btf_vlen(ta), btf_vlen(tb)); > + return false; > + } > + > + ma = btf_members(ta); > + mb = btf_members(tb); > + for (i = 0; i < btf_vlen(ta); i++, ma++, mb++) { > + const char *na, *nb; > + > + if (ma->offset != mb->offset) { > + pr_warn("%s:STRUCT field offset mismatch, (%u v.s. %u)\n", > + __func__, ma->offset, mb->offset); > + return false; > + } > + na = btf__name_by_offset(ba, ma->name_off); > + nb = btf__name_by_offset(bb, mb->name_off); > + if (strcmp(na, nb)) { > + pr_warn("%s:STRUCT field name mismatch, (%s v.s. %s)\n", > + __func__, na, nb); > + return false; > + } > + } I am wondering whether this is too strict and how this can co-work with CO-RE. Forcing users to write almost identical structure definition to the underlying kernel will not be user friendly and may not work cross kernel versions even if the field user cares have not changed. Maybe we can relax the constraint here. You can look at existing libbpf CO-RE code. > + break; > + } > + case BTF_KIND_ENUM: { /* compare vlen and member value, name */ > + const struct btf_enum *ma, *mb; > + > + if (btf_vlen(ta) != btf_vlen(tb)) { > + pr_warn("%s:ENUM vlen mismatch, (%u v.s. %u)\n", > + __func__, btf_vlen(ta), btf_vlen(tb)); > + return false; > + } > + > + ma = btf_enum(ta); > + mb = btf_enum(tb); > + for (i = 0; i < btf_vlen(ta); i++, ma++, mb++) { > + if (ma->val != mb->val) { > + pr_warn("%s:ENUM val mismatch, (%u v.s. %u)\n", > + __func__, ma->val, mb->val); > + return false; > + } > + } > + break; > + } > + case BTF_KIND_PTR: { /* naive compare of ref type for PTR */ > + if (!btf_ksym_equal_type(ba, ta->type, bb, tb->type)) { > + pr_warn("%s:PTR ref type mismatch.\n", __func__); > + return false; > + } > + break; > + } > + case BTF_KIND_FUNC_PROTO: { /* naive compare of vlen and param types */ > + const struct btf_param *pa, *pb; > + > + if (btf_vlen(ta) != btf_vlen(tb)) { > + pr_warn("%s:FUNC_PROTO vlen mismatch, (%u v.s. %u)\n", > + __func__, btf_vlen(ta), btf_vlen(tb)); > + return false; > + } > + > + pa = btf_params(ta); > + pb = btf_params(tb); > + for (i = 0; i < btf_vlen(ta); i++, pa++, pb++) { > + if (!btf_ksym_equal_type(ba, pa->type, bb, pb->type)) { > + pr_warn("%s:FUNC_PROTO params type mismatch.\n", > + __func__); > + return false; > + } > + } > + break; > + } > + case BTF_KIND_FUNC: > + case BTF_KIND_CONST: > + case BTF_KIND_VOLATILE: > + case BTF_KIND_RESTRICT: > + case BTF_KIND_TYPEDEF: > + case BTF_KIND_VAR: > + case BTF_KIND_DATASEC: > + pr_warn("unexpected type for matching ksym types.\n"); > + return false; > + default: > + pr_warn("unsupported btf types.\n"); > + return false; > + } > + > + return true; > +} > + > struct btf_ext_sec_setup_param { > __u32 off; > __u32 len; > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h > index 91f0ad0e0325..5ef220e52485 100644 > --- a/tools/lib/bpf/btf.h > +++ b/tools/lib/bpf/btf.h > @@ -52,6 +52,8 @@ LIBBPF_API int btf__get_map_kv_tids(const struct btf *btf, const char *map_name, > __u32 expected_key_size, > __u32 expected_value_size, > __u32 *key_type_id, __u32 *value_type_id); > +LIBBPF_API bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, > + const struct btf *bb, __u32 id_b); > > LIBBPF_API struct btf_ext *btf_ext__new(__u8 *data, __u32 size); > LIBBPF_API void btf_ext__free(struct btf_ext *btf_ext); The new API function should be added to libbpf.map.
On Thu, Aug 20, 2020 at 10:22 AM Yonghong Song <yhs@fb.com> wrote: > > > > On 8/19/20 3:40 PM, Hao Luo wrote: > > For a ksym to be safely dereferenced and accessed, its type defined in > > bpf program should basically match its type defined in kernel. Implement > > a help function for a quick matching, which is used by libbpf when > > resolving the kernel btf_id of a ksym. > > > > Signed-off-by: Hao Luo <haoluo@google.com> > > --- > > tools/lib/bpf/btf.c | 171 ++++++++++++++++++++++++++++++++++++++++++++ > > tools/lib/bpf/btf.h | 2 + > > 2 files changed, 173 insertions(+) > > > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > > index a3d259e614b0..2ff31f244d7a 100644 > > --- a/tools/lib/bpf/btf.c > > +++ b/tools/lib/bpf/btf.c > > @@ -1005,6 +1005,177 @@ int btf__get_map_kv_tids(const struct btf *btf, const char *map_name, > > return 0; > > } > > > > +/* > > + * Basic type check for ksym support. Only checks type kind and resolved size. > > + */ > > +static inline > > +bool btf_ksym_equal_type(const struct btf *ba, __u32 type_a, > > + const struct btf *bb, __u32 type_b) > > "ba" and "bb" is not descriptive. Maybe "btf_a" or "btf_b"? > or even "btf1" or "btf2" since the number does not carry > extra meaning compared to letters. > > The same for below, may be t1, t2? > > > +{ > > + const struct btf_type *ta, *tb; > > + > > + ta = btf__type_by_id(ba, type_a); > > + tb = btf__type_by_id(bb, type_b); > > + > > + /* compare type kind */ > > + if (btf_kind(ta) != btf_kind(tb)) > > + return false; > > + > > + /* compare resolved type size */ > > + return btf__resolve_size(ba, type_a) == btf__resolve_size(bb, type_b); > > +} > > + > > +/* > > + * Match a ksym's type defined in bpf programs against its type encoded in > > + * kernel btf. > > + */ > > +bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, > > + const struct btf *bb, __u32 id_b) > > +{ [...] > > + } > > + } > > I am wondering whether this is too strict and how this can co-work with > CO-RE. Forcing users to write almost identical structure definition to > the underlying kernel will not be user friendly and may not work cross > kernel versions even if the field user cares have not changed. > > Maybe we can relax the constraint here. You can look at existing > libbpf CO-RE code. Right. Hao, can you just re-use bpf_core_types_are_compat() instead? See if semantics makes sense, but I think it should. BPF CO-RE has been permissive in terms of struct size and few other type aspects, because it handles relocations so well. This approach allows to not have to exactly match all possible variations of some struct definition, which is a big problem with ever-changing kernel data structures. > > > + break; > > + } [...] > > + > > struct btf_ext_sec_setup_param { > > __u32 off; > > __u32 len; > > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h > > index 91f0ad0e0325..5ef220e52485 100644 > > --- a/tools/lib/bpf/btf.h > > +++ b/tools/lib/bpf/btf.h > > @@ -52,6 +52,8 @@ LIBBPF_API int btf__get_map_kv_tids(const struct btf *btf, const char *map_name, > > __u32 expected_key_size, > > __u32 expected_value_size, > > __u32 *key_type_id, __u32 *value_type_id); > > +LIBBPF_API bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, > > + const struct btf *bb, __u32 id_b); > > > > LIBBPF_API struct btf_ext *btf_ext__new(__u8 *data, __u32 size); > > LIBBPF_API void btf_ext__free(struct btf_ext *btf_ext); > > The new API function should be added to libbpf.map. My question is why does this even have to be a public API?
On Fri, Aug 21, 2020 at 2:50 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Aug 20, 2020 at 10:22 AM Yonghong Song <yhs@fb.com> wrote: > > > > > > > > On 8/19/20 3:40 PM, Hao Luo wrote: > > > For a ksym to be safely dereferenced and accessed, its type defined in > > > bpf program should basically match its type defined in kernel. Implement > > > a help function for a quick matching, which is used by libbpf when > > > resolving the kernel btf_id of a ksym. > > > > > > Signed-off-by: Hao Luo <haoluo@google.com> > > > --- [...] > > > +/* > > > + * Match a ksym's type defined in bpf programs against its type encoded in > > > + * kernel btf. > > > + */ > > > +bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, > > > + const struct btf *bb, __u32 id_b) > > > +{ > > [...] > > > > + } > > > + } > > > > I am wondering whether this is too strict and how this can co-work with > > CO-RE. Forcing users to write almost identical structure definition to > > the underlying kernel will not be user friendly and may not work cross > > kernel versions even if the field user cares have not changed. > > > > Maybe we can relax the constraint here. You can look at existing > > libbpf CO-RE code. > > Right. Hao, can you just re-use bpf_core_types_are_compat() instead? > See if semantics makes sense, but I think it should. BPF CO-RE has > been permissive in terms of struct size and few other type aspects, > because it handles relocations so well. This approach allows to not > have to exactly match all possible variations of some struct > definition, which is a big problem with ever-changing kernel data > structures. > I have to say I hate myself writing another type comparison instead of reusing the existing one. The issue is that when bpf_core_types_compat compares names, it uses t1->name_off == t2->name_off. It is also used in bpf_equal_common(). In my case, because these types are from two different BTFs, their name_off are not expected to be the same, right? I didn't find a good solution to refactor before posting this patch. I think I can adapt bpf_core_type_compat() and pay more attention to CO-RE. > > > > > + break; > > > + } > > [...] > > > > + > > > struct btf_ext_sec_setup_param { > > > __u32 off; > > > __u32 len; > > > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h > > > index 91f0ad0e0325..5ef220e52485 100644 > > > --- a/tools/lib/bpf/btf.h > > > +++ b/tools/lib/bpf/btf.h > > > @@ -52,6 +52,8 @@ LIBBPF_API int btf__get_map_kv_tids(const struct btf *btf, const char *map_name, > > > __u32 expected_key_size, > > > __u32 expected_value_size, > > > __u32 *key_type_id, __u32 *value_type_id); > > > +LIBBPF_API bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, > > > + const struct btf *bb, __u32 id_b); > > > > > > LIBBPF_API struct btf_ext *btf_ext__new(__u8 *data, __u32 size); > > > LIBBPF_API void btf_ext__free(struct btf_ext *btf_ext); > > > > The new API function should be added to libbpf.map. > > My question is why does this even have to be a public API? I can fix. Please pardon my ignorance, what is the difference between public and internal APIs? I wasn't sure, so used it improperly. Thanks, Hao
On Fri, Aug 21, 2020 at 5:43 PM Hao Luo <haoluo@google.com> wrote: > > On Fri, Aug 21, 2020 at 2:50 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Thu, Aug 20, 2020 at 10:22 AM Yonghong Song <yhs@fb.com> wrote: > > > > > > > > > > > > On 8/19/20 3:40 PM, Hao Luo wrote: > > > > For a ksym to be safely dereferenced and accessed, its type defined in > > > > bpf program should basically match its type defined in kernel. Implement > > > > a help function for a quick matching, which is used by libbpf when > > > > resolving the kernel btf_id of a ksym. > > > > > > > > Signed-off-by: Hao Luo <haoluo@google.com> > > > > --- > [...] > > > > +/* > > > > + * Match a ksym's type defined in bpf programs against its type encoded in > > > > + * kernel btf. > > > > + */ > > > > +bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, > > > > + const struct btf *bb, __u32 id_b) > > > > +{ > > > > [...] > > > > > > + } > > > > + } > > > > > > I am wondering whether this is too strict and how this can co-work with > > > CO-RE. Forcing users to write almost identical structure definition to > > > the underlying kernel will not be user friendly and may not work cross > > > kernel versions even if the field user cares have not changed. > > > > > > Maybe we can relax the constraint here. You can look at existing > > > libbpf CO-RE code. > > > > Right. Hao, can you just re-use bpf_core_types_are_compat() instead? > > See if semantics makes sense, but I think it should. BPF CO-RE has > > been permissive in terms of struct size and few other type aspects, > > because it handles relocations so well. This approach allows to not > > have to exactly match all possible variations of some struct > > definition, which is a big problem with ever-changing kernel data > > structures. > > > > I have to say I hate myself writing another type comparison instead of > reusing the existing one. The issue is that when bpf_core_types_compat > compares names, it uses t1->name_off == t2->name_off. It is also used Huh? Are we talking about the same bpf_core_types_are_compat() (there is no bpf_core_types_compat, I think it's a typo)? bpf_core_types_are_compat() doesn't even compare any name, so I'm not sure what you are talking about. Some of btf_dedup functions do string comparisons using name_off directly, but that's a special and very careful case, it's not relevant here. > in bpf_equal_common(). In my case, because these types are from two > different BTFs, their name_off are not expected to be the same, right? > I didn't find a good solution to refactor before posting this patch. I bpf_core_types_are_compat() didn't land until this week, so you must be confusing something. Please take another look. > think I can adapt bpf_core_type_compat() and pay more attention to > CO-RE. > > > > > > > > + break; > > > > + } > > > > [...] > > > > > > + > > > > struct btf_ext_sec_setup_param { > > > > __u32 off; > > > > __u32 len; > > > > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h > > > > index 91f0ad0e0325..5ef220e52485 100644 > > > > --- a/tools/lib/bpf/btf.h > > > > +++ b/tools/lib/bpf/btf.h > > > > @@ -52,6 +52,8 @@ LIBBPF_API int btf__get_map_kv_tids(const struct btf *btf, const char *map_name, > > > > __u32 expected_key_size, > > > > __u32 expected_value_size, > > > > __u32 *key_type_id, __u32 *value_type_id); > > > > +LIBBPF_API bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, > > > > + const struct btf *bb, __u32 id_b); > > > > > > > > LIBBPF_API struct btf_ext *btf_ext__new(__u8 *data, __u32 size); > > > > LIBBPF_API void btf_ext__free(struct btf_ext *btf_ext); > > > > > > The new API function should be added to libbpf.map. > > > > My question is why does this even have to be a public API? > > I can fix. Please pardon my ignorance, what is the difference between > public and internal APIs? I wasn't sure, so used it improperly. public APIs are those that users of libbpf are supposed to use, internal one is just for libbpf internal use. The former can't change, the latter can be refactor as much as we need to. > > Thanks, > Hao
Ah, I see bpf_core_types_are_compat() after sync'ing my local repo. It seems the perfect fit for my use case. I only found the btf_equal_xxx() defined in btf.c when posting these patches. I can test and use bpf_core_types_are_compat() in v2. Thanks for pointing it out and explaining the public APIs. Hao On Fri, Aug 21, 2020 at 7:43 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Aug 21, 2020 at 5:43 PM Hao Luo <haoluo@google.com> wrote: > > > > On Fri, Aug 21, 2020 at 2:50 PM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Thu, Aug 20, 2020 at 10:22 AM Yonghong Song <yhs@fb.com> wrote: > > > > > > > > > > > > > > > > On 8/19/20 3:40 PM, Hao Luo wrote: > > > > > For a ksym to be safely dereferenced and accessed, its type defined in > > > > > bpf program should basically match its type defined in kernel. Implement > > > > > a help function for a quick matching, which is used by libbpf when > > > > > resolving the kernel btf_id of a ksym. > > > > > > > > > > Signed-off-by: Hao Luo <haoluo@google.com> > > > > > --- > > [...] > > > > > +/* > > > > > + * Match a ksym's type defined in bpf programs against its type encoded in > > > > > + * kernel btf. > > > > > + */ > > > > > +bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, > > > > > + const struct btf *bb, __u32 id_b) > > > > > +{ > > > > > > [...] > > > > > > > > + } > > > > > + } > > > > > > > > I am wondering whether this is too strict and how this can co-work with > > > > CO-RE. Forcing users to write almost identical structure definition to > > > > the underlying kernel will not be user friendly and may not work cross > > > > kernel versions even if the field user cares have not changed. > > > > > > > > Maybe we can relax the constraint here. You can look at existing > > > > libbpf CO-RE code. > > > > > > Right. Hao, can you just re-use bpf_core_types_are_compat() instead? > > > See if semantics makes sense, but I think it should. BPF CO-RE has > > > been permissive in terms of struct size and few other type aspects, > > > because it handles relocations so well. This approach allows to not > > > have to exactly match all possible variations of some struct > > > definition, which is a big problem with ever-changing kernel data > > > structures. > > > > > > > I have to say I hate myself writing another type comparison instead of > > reusing the existing one. The issue is that when bpf_core_types_compat > > compares names, it uses t1->name_off == t2->name_off. It is also used > > Huh? Are we talking about the same bpf_core_types_are_compat() (there > is no bpf_core_types_compat, I think it's a typo)? > bpf_core_types_are_compat() doesn't even compare any name, so I'm not > sure what you are talking about. Some of btf_dedup functions do string > comparisons using name_off directly, but that's a special and very > careful case, it's not relevant here. > > > > in bpf_equal_common(). In my case, because these types are from two > > different BTFs, their name_off are not expected to be the same, right? > > I didn't find a good solution to refactor before posting this patch. I > > bpf_core_types_are_compat() didn't land until this week, so you must > be confusing something. Please take another look. > > > think I can adapt bpf_core_type_compat() and pay more attention to > > CO-RE. > > > > > > > > > > > + break; > > > > > + } > > > > > > [...] > > > > > > > > + > > > > > struct btf_ext_sec_setup_param { > > > > > __u32 off; > > > > > __u32 len; > > > > > diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h > > > > > index 91f0ad0e0325..5ef220e52485 100644 > > > > > --- a/tools/lib/bpf/btf.h > > > > > +++ b/tools/lib/bpf/btf.h > > > > > @@ -52,6 +52,8 @@ LIBBPF_API int btf__get_map_kv_tids(const struct btf *btf, const char *map_name, > > > > > __u32 expected_key_size, > > > > > __u32 expected_value_size, > > > > > __u32 *key_type_id, __u32 *value_type_id); > > > > > +LIBBPF_API bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, > > > > > + const struct btf *bb, __u32 id_b); > > > > > > > > > > LIBBPF_API struct btf_ext *btf_ext__new(__u8 *data, __u32 size); > > > > > LIBBPF_API void btf_ext__free(struct btf_ext *btf_ext); > > > > > > > > The new API function should be added to libbpf.map. > > > > > > My question is why does this even have to be a public API? > > > > I can fix. Please pardon my ignorance, what is the difference between > > public and internal APIs? I wasn't sure, so used it improperly. > > public APIs are those that users of libbpf are supposed to use, > internal one is just for libbpf internal use. The former can't change, > the latter can be refactor as much as we need to. > > > > > Thanks, > > Hao
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c index a3d259e614b0..2ff31f244d7a 100644 --- a/tools/lib/bpf/btf.c +++ b/tools/lib/bpf/btf.c @@ -1005,6 +1005,177 @@ int btf__get_map_kv_tids(const struct btf *btf, const char *map_name, return 0; } +/* + * Basic type check for ksym support. Only checks type kind and resolved size. + */ +static inline +bool btf_ksym_equal_type(const struct btf *ba, __u32 type_a, + const struct btf *bb, __u32 type_b) +{ + const struct btf_type *ta, *tb; + + ta = btf__type_by_id(ba, type_a); + tb = btf__type_by_id(bb, type_b); + + /* compare type kind */ + if (btf_kind(ta) != btf_kind(tb)) + return false; + + /* compare resolved type size */ + return btf__resolve_size(ba, type_a) == btf__resolve_size(bb, type_b); +} + +/* + * Match a ksym's type defined in bpf programs against its type encoded in + * kernel btf. + */ +bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, + const struct btf *bb, __u32 id_b) +{ + const struct btf_type *ta = btf__type_by_id(ba, id_a); + const struct btf_type *tb = btf__type_by_id(bb, id_b); + int i; + + /* compare type kind */ + if (btf_kind(ta) != btf_kind(tb)) { + pr_warn("%s:mismatched type kind (%d v.s. %d).\n", + __func__, btf_kind(ta), btf_kind(tb)); + return false; + } + + switch (btf_kind(ta)) { + case BTF_KIND_INT: { /* compare size and encoding */ + __u32 ea, eb; + + if (ta->size != tb->size) { + pr_warn("%s:INT size mismatch, (%u v.s. %u)\n", + __func__, ta->size, tb->size); + return false; + } + ea = *(__u32 *)(ta + 1); + eb = *(__u32 *)(tb + 1); + if (ea != eb) { + pr_warn("%s:INT encoding mismatch (%u v.s. %u)\n", + __func__, ea, eb); + return false; + } + break; + } + case BTF_KIND_ARRAY: { /* compare type and number of elements */ + const struct btf_array *ea, *eb; + + ea = btf_array(ta); + eb = btf_array(tb); + if (!btf_ksym_equal_type(ba, ea->type, bb, eb->type)) { + pr_warn("%s:ARRAY elem type mismatch.\n", __func__); + return false; + } + if (ea->nelems != eb->nelems) { + pr_warn("%s:ARRAY nelems mismatch (%d v.s. %d)\n", + __func__, ea->nelems, eb->nelems); + return false; + } + break; + } + case BTF_KIND_STRUCT: + case BTF_KIND_UNION: { /* compare size, vlen and member offset, name */ + const struct btf_member *ma, *mb; + + if (ta->size != tb->size) { + pr_warn("%s:STRUCT size mismatch, (%u v.s. %u)\n", + __func__, ta->size, tb->size); + return false; + } + if (btf_vlen(ta) != btf_vlen(tb)) { + pr_warn("%s:STRUCT vlen mismatch, (%u v.s. %u)\n", + __func__, btf_vlen(ta), btf_vlen(tb)); + return false; + } + + ma = btf_members(ta); + mb = btf_members(tb); + for (i = 0; i < btf_vlen(ta); i++, ma++, mb++) { + const char *na, *nb; + + if (ma->offset != mb->offset) { + pr_warn("%s:STRUCT field offset mismatch, (%u v.s. %u)\n", + __func__, ma->offset, mb->offset); + return false; + } + na = btf__name_by_offset(ba, ma->name_off); + nb = btf__name_by_offset(bb, mb->name_off); + if (strcmp(na, nb)) { + pr_warn("%s:STRUCT field name mismatch, (%s v.s. %s)\n", + __func__, na, nb); + return false; + } + } + break; + } + case BTF_KIND_ENUM: { /* compare vlen and member value, name */ + const struct btf_enum *ma, *mb; + + if (btf_vlen(ta) != btf_vlen(tb)) { + pr_warn("%s:ENUM vlen mismatch, (%u v.s. %u)\n", + __func__, btf_vlen(ta), btf_vlen(tb)); + return false; + } + + ma = btf_enum(ta); + mb = btf_enum(tb); + for (i = 0; i < btf_vlen(ta); i++, ma++, mb++) { + if (ma->val != mb->val) { + pr_warn("%s:ENUM val mismatch, (%u v.s. %u)\n", + __func__, ma->val, mb->val); + return false; + } + } + break; + } + case BTF_KIND_PTR: { /* naive compare of ref type for PTR */ + if (!btf_ksym_equal_type(ba, ta->type, bb, tb->type)) { + pr_warn("%s:PTR ref type mismatch.\n", __func__); + return false; + } + break; + } + case BTF_KIND_FUNC_PROTO: { /* naive compare of vlen and param types */ + const struct btf_param *pa, *pb; + + if (btf_vlen(ta) != btf_vlen(tb)) { + pr_warn("%s:FUNC_PROTO vlen mismatch, (%u v.s. %u)\n", + __func__, btf_vlen(ta), btf_vlen(tb)); + return false; + } + + pa = btf_params(ta); + pb = btf_params(tb); + for (i = 0; i < btf_vlen(ta); i++, pa++, pb++) { + if (!btf_ksym_equal_type(ba, pa->type, bb, pb->type)) { + pr_warn("%s:FUNC_PROTO params type mismatch.\n", + __func__); + return false; + } + } + break; + } + case BTF_KIND_FUNC: + case BTF_KIND_CONST: + case BTF_KIND_VOLATILE: + case BTF_KIND_RESTRICT: + case BTF_KIND_TYPEDEF: + case BTF_KIND_VAR: + case BTF_KIND_DATASEC: + pr_warn("unexpected type for matching ksym types.\n"); + return false; + default: + pr_warn("unsupported btf types.\n"); + return false; + } + + return true; +} + struct btf_ext_sec_setup_param { __u32 off; __u32 len; diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h index 91f0ad0e0325..5ef220e52485 100644 --- a/tools/lib/bpf/btf.h +++ b/tools/lib/bpf/btf.h @@ -52,6 +52,8 @@ LIBBPF_API int btf__get_map_kv_tids(const struct btf *btf, const char *map_name, __u32 expected_key_size, __u32 expected_value_size, __u32 *key_type_id, __u32 *value_type_id); +LIBBPF_API bool btf_ksym_type_match(const struct btf *ba, __u32 id_a, + const struct btf *bb, __u32 id_b); LIBBPF_API struct btf_ext *btf_ext__new(__u8 *data, __u32 size); LIBBPF_API void btf_ext__free(struct btf_ext *btf_ext);
For a ksym to be safely dereferenced and accessed, its type defined in bpf program should basically match its type defined in kernel. Implement a help function for a quick matching, which is used by libbpf when resolving the kernel btf_id of a ksym. Signed-off-by: Hao Luo <haoluo@google.com> --- tools/lib/bpf/btf.c | 171 ++++++++++++++++++++++++++++++++++++++++++++ tools/lib/bpf/btf.h | 2 + 2 files changed, 173 insertions(+)