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. > + 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? > + 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