Re: [PATCH bpf-next] libbpf: Return -EINVAL from btf__align_of() for unknown type ID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: btf__align_of() negative error breaks btf_dump.c
--

commit d6024f50b48c7fc7873ce247e9f420706729d50a
Author: Abhinav Madake <abhinavmadake@gmail.com>

libbpf: Return -EINVAL from btf__align_of() for unknown type ID

btf__align_of() passes the result of btf__type_by_id() straight to
btf_kind() without checking for NULL, so an unknown type ID
dereferences a NULL pointer instead of returning an error like the
other btf__*() lookups do.

BTF parsed from raw data cannot trigger this, since btf_sanity_check()
rejects dangling references. BTF built with the btf__add_*() API can:
those only check the ID range, so a forward reference to a type that
was never added, followed by btf__align_of() on it (directly, or
through the recursive struct/array/typedef cases), crashes.

Check for NULL and return -EINVAL, matching btf__type_by_id().

Link: https://github.com/libbpf/libbpf/issues/983

> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 908bd344229d5..d15bc09ac53de 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -986,8 +986,12 @@ __s64 btf__resolve_size(const struct btf *btf, __u32 type_id)
>  int btf__align_of(const struct btf *btf, __u32 id)
>  {
>  	const struct btf_type *t = btf__type_by_id(btf, id);
> -	__u16 kind = btf_kind(t);
> +	__u16 kind;
>  
> +	if (!t)
> +		return libbpf_err(-EINVAL);

[Severity: Medium]
Will this negative return value break callers in tools/lib/bpf/btf_dump.c
that expect 0 on error?

When btf_dump.c processes invalid BTF types, this -EINVAL return bypasses
checks for 0, leading to modulo operations with a negative number promoted
to a large unsigned integer.

In ptr_is_aligned(), the negative alignment bypasses the check for 0:

tools/lib/bpf/btf_dump.c:ptr_is_aligned() {
	int alignment = btf__align_of(btf, type_id);

	if (alignment == 0)
		return false;

	return ((uintptr_t)data) % alignment == 0;
}

In btf_is_struct_packed(), the error bypasses the false-check (which
expects 0), causing an invalid modulo operation with 8 * -22 promoted
to unsigned:

tools/lib/bpf/btf_dump.c:btf_is_struct_packed() {
    ...
		align = btf__align_of(btf, m->type);
		bit_sz = btf_member_bitfield_size(t, i);
		if (align && bit_sz == 0 && m->offset % (8 * align) != 0)
			return true;
    ...
}

Should the callers in btf_dump.c be updated to handle negative return
values?

> +
> +	kind = btf_kind(t);
>  	switch (kind) {
>  	case BTF_KIND_INT:
>  	case BTF_KIND_ENUM:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919164229.95154-1-abhinavmadake@gmail.com?part=1