Re: [PATCH bpf] libbpf: Fix type size overflow in btf_type_size_unknown

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] This problem wasn't introduced by this patch, but the patch attempts to fix a pointer wrap-around vulnerability and leaves it completely unfixed for standard BTF kinds.
- [Medium] The bounds check added in btf_type_size_unknown() performs pointer subtraction between unrelated memory allocations, leading to C Undefined Behavior.
--

commit ac478ac510f0298a2cc2458f409eff3e04fdd3b5
Author: Alan Maguire <alan.maguire@oracle.com>

libbpf: Fix type size overflow in btf_type_size_unknown

This commit attempts to fix a 32-bit pointer arithmetic wrap-around in
btf_parse_type_sec() by adding a bounds check in btf_type_size_unknown().
This aims to prevent a large type_size from wrapping when added to next_type,
which would bypass the existing bounds checks.

Link: https://lore.kernel.org/bpf/20260417170712.74E77C19425@smtp.kernel.org/

> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index ceb57b46a8782..d48ac5460d1de 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -420,9 +420,11 @@ static int btf_parse_layout_sec(struct btf *btf)
>  static int btf_type_size_unknown(const struct btf *btf, const struct btf_type *t)
>  {
>  	__u32 l_cnt = btf->hdr.layout_len / sizeof(struct btf_layout);
> +	const void *end_type = btf->types_data + btf->hdr.type_len;
>  	struct btf_layout *l = btf->layout;
>  	__u16 vlen = btf_vlen(t);
>  	__u32 kind = btf_kind(t);
> +	int type_size;
>  
>  	/* Fall back to base BTF if needed as they share layout information */
>  	if (!l) {

[ ... ]

> @@ -448,7 +450,13 @@ static int btf_type_size_unknown(const struct btf *btf, const struct btf_type *t
>  		return -EINVAL;
>  	}
>  
> -	return sizeof(struct btf_type) + l[kind].info_sz + vlen * l[kind].elem_sz;
> +	type_size = sizeof(struct btf_type) + l[kind].info_sz + vlen * l[kind].elem_sz;
> +	if ((size_t)type_size > (size_t)(end_type - (const void *)t)) {

Is it safe to subtract these pointers? When btf_type_size_unknown() is used
outside of parsing, such as in btf_dedup(), t might point to a newly
allocated new_types buffer while end_type points to the old btf->types_data.

Subtracting pointers from distinct memory allocations is undefined behavior
in C, which can cause crashes if tools linking libbpf are compiled with UBSan
using -fsanitize=pointer-subtract. Should this bounds check be relocated
into btf_parse_type_sec() instead?

> +		pr_debug("Overflow in type size %d for kind %u\n",
> +			 type_size, kind);
> +		return -EINVAL;
> +	}
> +	return type_size;
>  }

This patch attempts to fix a pointer wrap-around vulnerability, but does this
approach leave standard BTF kinds vulnerable?

Since the bounds check is only added to btf_type_size_unknown(), standard
kinds like BTF_KIND_STRUCT are evaluated directly in btf_type_size() without
any bounds check.

If a malicious BTF file provides a BTF_KIND_STRUCT with a maximum vlen, it
can yield a type_size up to ~786KB. On a 32-bit system, if the buffer is
allocated near the top of the address space, adding this size could wrap
around to a small value.

The overflow check next_type + type_size > end_type in btf_parse_type_sec()
would then evaluate to false, potentially bypassing validation and leading to
an out-of-bounds write in btf_bswap_type_rest().

Would it be safer to fix the vulnerability for all kinds by changing the
check in btf_parse_type_sec() to avoid pointer addition entirely?

For example:
if ((size_t)type_size > (size_t)(end_type - next_type))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260422103206.533695-1-alan.maguire@oracle.com?part=1