Re: [PATCHv5 bpf-next 19/28] libbpf: Add btf_type_is_traceable_func function

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [High] Unsafe BTF type resolution in `btf_type_is_traceable_func` lacks validation and bounds checking, leading to out-of-bounds read and segfault.
- [Medium] `btf_get_type_size` hardcodes `sizeof(void *)` for pointer sizes, breaking cross-architecture analysis.
- [Medium] Missing maximum resolution depth in type modifier loop allows Denial of Service via infinite loop.
--

commit 70d9630993310d9569ee7d038356f4a4ec3a18e9
Author: Jiri Olsa <jolsa@kernel.org>

libbpf: Add btf_type_is_traceable_func function

Adds the btf_type_is_traceable_func function to perform checks on functions
to prevent attachment on some functions. Exports it via libbpf_internal.h
for use by benchmark tests.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 3a80a018fc7d5..65a9ac10053d5 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[ ... ]

> @@ -12425,6 +12425,80 @@ static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, stru
>  	return ret;
>  }
>  
> +#define MAX_BPF_FUNC_ARGS 12
> +
> +static bool btf_type_is_modifier(const struct btf_type *t)
> +{
> +	switch (BTF_INFO_KIND(t->info)) {
> +	case BTF_KIND_TYPEDEF:
> +	case BTF_KIND_VOLATILE:
> +	case BTF_KIND_CONST:
> +	case BTF_KIND_RESTRICT:
> +	case BTF_KIND_TYPE_TAG:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static int btf_get_type_size(const struct btf *btf, __u32 type_id,
> +			     const struct btf_type **ret_type)
> +{
> +	const struct btf_type *t;
> +
> +	*ret_type = btf__type_by_id(btf, 0);
> +	if (!type_id)
> +		return 0;
> +	t = btf__type_by_id(btf, type_id);
> +	while (t && btf_type_is_modifier(t))
> +		t = btf__type_by_id(btf, t->type);

Does this loop need a maximum resolution depth limit?

If a malformed BTF object contains a circular modifier reference (e.g.,
typedef A -> const B -> typedef A), could this loop spin infinitely and
cause a hang?

> +	if (!t)
> +		return -EINVAL;
> +	*ret_type = t;
> +	if (btf_is_ptr(t))
> +		return sizeof(void *);

Does this correctly calculate the pointer size for cross-architecture
analysis?

Since libbpf is a user-space library that may process BTF blobs from
different architectures (like a 64-bit tool processing a 32-bit BPF
object), will hardcoding sizeof(void *) return the host's pointer size
instead of the target's pointer size?

Would using btf__pointer_size(btf) be more appropriate here?

> +	if (btf_is_int(t) || btf_is_any_enum(t) || btf_is_struct(t) || btf_is_union(t))
> +		return t->size;
> +	return -EINVAL;
> +}
> +
> +bool btf_type_is_traceable_func(const struct btf *btf, const struct btf_type *t)
> +{
> +	const struct btf_param *args;
> +	const struct btf_type *proto;
> +	__u32 i, nargs;
> +	int ret;
> +
> +	proto = btf_type_by_id(btf, t->type);

Could this cause an out-of-bounds read if t is not a function type?

Because the type and size fields share a union in struct btf_type, passing
a type like BTF_KIND_STRUCT causes its size to be misinterpreted as a type
ID. The internal btf_type_by_id() helper skips bounds checking.

If the interpreted ID exceeds the BTF type array bounds, it might read past
the end of the array and return a garbage pointer, which would then be
dereferenced in the next line.

Should this verify !btf_is_func(t) first, and use the bounds-checked public
API btf__type_by_id() along with a NULL check for proto?

> +	if (BTF_INFO_KIND(proto->info) != BTF_KIND_FUNC_PROTO)
> +		return false;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260417192502.194548-20-jolsa@kernel.org?part=1