Re: [PATCH bpf-next] libbpf: add basic BTF sanity validation

[Eduard Zingerman <eddyz87@gmail.com>]

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@kernel.org>
> ---
>  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