On Thu, Aug 24, 2023 at 5:11 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Wed, 2023-08-23 at 16:44 -0700, Andrii Nakryiko wrote: > > Implement a simple and straightforward BTF sanity check when loading BTF > > information for BPF ELF file. Right now it's very basic and just > > validates that all the string offsets and type IDs are within valid > > range. But even with such simple checks it fixes a bunch of crashes > > found by OSS fuzzer ([0]-[5]) and will allow fuzzer to make further > > progress. > > > > Some other invariants will be checked in follow up patches (like > > ensuring there is no infinite type loops), but this seems like a good > > start already. > > > > [0] https://github.com/libbpf/libbpf/issues/482 > > [1] https://github.com/libbpf/libbpf/issues/483 > > [2] https://github.com/libbpf/libbpf/issues/485 > > [3] https://github.com/libbpf/libbpf/issues/613 > > [4] https://github.com/libbpf/libbpf/issues/618 > > [5] https://github.com/libbpf/libbpf/issues/619 > > > > Closes: https://github.com/libbpf/libbpf/issues/617 > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > --- > > tools/lib/bpf/btf.c | 146 ++++++++++++++++++++++++++++++++ > > tools/lib/bpf/libbpf.c | 7 ++ > > tools/lib/bpf/libbpf_internal.h | 2 + > > 3 files changed, 155 insertions(+) > > > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > > index 8484b563b53d..5f23df94861e 100644 > > --- a/tools/lib/bpf/btf.c > > +++ b/tools/lib/bpf/btf.c > > @@ -1155,6 +1155,152 @@ struct btf *btf__parse_split(const char *path, struct btf *base_btf) > > return libbpf_ptr(btf_parse(path, base_btf, NULL)); > > } > > > > +static int btf_validate_str(const struct btf *btf, __u32 str_off, const char *what, __u32 type_id) > > +{ > > + const char *s; > > + > > + s = btf__str_by_offset(btf, str_off); > > + if (!s) { > > + pr_warn("btf: type [%u]: invalid %s (string offset %u)\n", type_id, what, str_off); > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +static int btf_validate_id(const struct btf *btf, __u32 id, __u32 ctx_id) > > +{ > > + const struct btf_type *t; > > + > > + t = btf__type_by_id(btf, id); > > + if (!t) { > > + pr_warn("btf: type [%u]: invalid referenced type ID %u\n", ctx_id, id); > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +static int btf_validate_type(const struct btf *btf, const struct btf_type *t, __u32 id) > > +{ > > + __u32 kind = btf_kind(t); > > + int err, i, n; > > + > > + err = btf_validate_str(btf, t->name_off, "type name", id); > > + if (err) > > + return err; > > + > > + switch (kind) { > > + case BTF_KIND_UNKN: > > + case BTF_KIND_INT: > > + case BTF_KIND_FWD: > > + case BTF_KIND_FLOAT: > > + break; > > + case BTF_KIND_PTR: > > + case BTF_KIND_TYPEDEF: > > + case BTF_KIND_VOLATILE: > > + case BTF_KIND_CONST: > > + case BTF_KIND_RESTRICT: > > + case BTF_KIND_FUNC: > > + case BTF_KIND_VAR: > > + case BTF_KIND_DECL_TAG: > > + case BTF_KIND_TYPE_TAG: > > + err = btf_validate_id(btf, t->type, id); > > + if (err) > > + return err; > > + break; > > + case BTF_KIND_ARRAY: { > > + const struct btf_array *a = btf_array(t); > > + > > + err = btf_validate_id(btf, a->index_type, id); > > `a->index_type` should probably be `a->type` here, otherwise these two > checks are identical. copy/paste error, nice catch, will fix > > > + err = err ?: btf_validate_id(btf, a->index_type, id); > > + if (err) > > + return err; > > + break; > > + } > > + case BTF_KIND_STRUCT: > > + case BTF_KIND_UNION: { > > + const struct btf_member *m = btf_members(t); > > + > > + n = btf_vlen(t); > > + for (i = 0; i < n; i++, m++) { > > + err = btf_validate_str(btf, m->name_off, "field name", id); > > + err = err ?: btf_validate_id(btf, m->type, id); > > + if (err) > > + return err; > > + } > > + break; > > + } > > + case BTF_KIND_ENUM: { > > + const struct btf_enum *m = btf_enum(t); > > + > > + n = btf_vlen(t); > > + for (i = 0; i < n; i++, m++) { > > + err = btf_validate_str(btf, m->name_off, "enum name", id); > > + if (err) > > + return err; > > + } > > + break; > > + } > > + case BTF_KIND_ENUM64: { > > + const struct btf_enum64 *m = btf_enum64(t); > > + > > + n = btf_vlen(t); > > + for (i = 0; i < n; i++, m++) { > > + err = btf_validate_str(btf, m->name_off, "enum name", id); > > + if (err) > > + return err; > > + } > > + break; > > + } > > + case BTF_KIND_FUNC_PROTO: { > > + const struct btf_param *m = btf_params(t); > > + > > + n = btf_vlen(t); > > + for (i = 0; i < n; i++, m++) { > > + err = btf_validate_str(btf, m->name_off, "param name", id); > > Maybe check `m->type` here as well? of course, I suspected I missed something obvious :) > > > + if (err) > > + return err; > > + } > > + break; > > + } > > + case BTF_KIND_DATASEC: { > > + const struct btf_var_secinfo *m = btf_var_secinfos(t); > > + > > + n = btf_vlen(t); > > + for (i = 0; i < n; i++, m++) { > > + err = btf_validate_id(btf, m->type, id); > > + if (err) > > + return err; > > + } > > + break; > > + } > > + default: > > + pr_warn("btf: type [%u]: unrecognized kind %u\n", id, kind); > > + return -EINVAL; > > + } > > + return 0; > > +} > > + > > +/* Validate basic sanity of BTF. It's intentionally less thorough than > > + * kernel's validation and validates only properties of BTF that libbpf relies > > + * on to be correct (e.g., valid type IDs, valid string offsets, etc) > > + */ > > +int btf_sanity_check(const struct btf *btf) > > +{ > > + const struct btf_type *t; > > + __u32 i, n = btf__type_cnt(btf); > > + int err; > > + > > + for (i = 1; i < n; i++) { > > + t = btf_type_by_id(btf, i); > > + err = btf_validate_type(btf, t, i); > > + if (err) > > + return err; > > + } > > + return 0; > > +} > > + > > static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endian); > > > > int btf_load_into_kernel(struct btf *btf, char *log_buf, size_t log_sz, __u32 log_level) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > index 4c3967d94b6d..71a3c768d9af 100644 > > --- a/tools/lib/bpf/libbpf.c > > +++ b/tools/lib/bpf/libbpf.c > > @@ -2833,6 +2833,13 @@ static int bpf_object__init_btf(struct bpf_object *obj, > > pr_warn("Error loading ELF section %s: %d.\n", BTF_ELF_SEC, err); > > goto out; > > } > > + err = btf_sanity_check(obj->btf); > > + if (err) { > > + pr_warn("elf: .BTF data is corrupted, discarding it...\n"); > > + btf__free(obj->btf); > > + obj->btf = NULL; > > + goto out; > > + } > > /* enforce 8-byte pointers for BPF-targeted BTFs */ > > btf__set_pointer_size(obj->btf, 8); > > } > > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h > > index f0f08635adb0..5e715a2d48f2 100644 > > --- a/tools/lib/bpf/libbpf_internal.h > > +++ b/tools/lib/bpf/libbpf_internal.h > > @@ -76,6 +76,8 @@ > > #define BTF_TYPE_TYPE_TAG_ENC(value, type) \ > > BTF_TYPE_ENC(value, BTF_INFO_ENC(BTF_KIND_TYPE_TAG, 0, 0), type) > > > > +int btf_sanity_check(const struct btf *btf); > > + > > #ifndef likely > > #define likely(x) __builtin_expect(!!(x), 1) > > #endif >
