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

On Sat, Apr 18, 2026 at 03:40:48AM +0000, [email protected] wrote:
> 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 <[email protected]>
> 
> 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?

I will add that, something like in btf__resolve_type

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

yep, will change

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

will add the check

jirka