Re: [PATCH v3 bpf-next 2/6] libbpf: Adjust btf_vlen() to return a __u32

On 17/04/2026 18:07, [email protected] wrote:
> Sashiko AI review found 1 potential issue(s):
> - [High] On 32-bit systems, `btf_parse_type_sec()` is vulnerable to pointer arithmetic wrap-around due to missing strict bounds checking, allowing an attacker-crafted BTF layout to bypass safety checks and cause out-of-bounds memory access.
> --
> 
> commit 0d6d9306686384cb94334ae8845f954db44a9f74
> Author: Alan Maguire <[email protected]>
> 
> libbpf: Adjust btf_vlen() to return a __u32
> 
> Now that vlen is 24 bits, btf_vlen() must return a __u32.
> Adjust use cases in libbpf accordingly.  Also add error
> handling to avoid vlen overflow in btf_type_inc_vlen().
> 
>> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
>> index ceb57b46a8782..267904939098c 100644
>> --- a/tools/lib/bpf/btf.c
>> +++ b/tools/lib/bpf/btf.c
>> @@ -421,7 +421,7 @@ 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);
>>  	struct btf_layout *l = btf->layout;
>> -	__u16 vlen = btf_vlen(t);
>> +	__u32 vlen = btf_vlen(t);
>>  	__u32 kind = btf_kind(t);
> 
> Can this lead to a pointer arithmetic wrap-around regression on 32-bit
> systems?
> 
> The size is calculated later in this function using unsigned 32-bit
> arithmetic and returned as a signed int:
> 
>     return sizeof(struct btf_type) + l[kind].info_sz + vlen * l[kind].elem_sz;
> 
> An attacker could provide a custom layout with an artificially large elem_sz
> (e.g., 2,000,000,000). For a type with vlen = 1, the calculated size fits
> within a positive signed 32-bit int and is returned as a valid positive
> type_size.
> 
> When btf_parse_type_sec() checks if next_type + type_size > end_type, it
> evaluates this using 32-bit pointer arithmetic. If next_type is located high
> in the address space, adding 2,000,000,000 wraps around the 32-bit boundary
> to a numerically smaller low address, incorrectly passing the bounds check.
> 
> This causes next_type to advance to the wrapped address, allowing subsequent
> loop iterations to read arbitrary memory and potentially cause out-of-bounds
> accesses or crashes.
> 
> While this parser logic issue predates this patch, should we address it here
> by safely checking the offset instead? For example:
> 
>     if ((size_t)type_size > (size_t)((char *)end_type - (char *)next_type))
> 

Ok so I think the suggestion is the following in btf_parse_type_sec() is the problem:

		type_size = btf_type_size(btf, next_type);
                if (type_size < 0)
                        return type_size;
                if (next_type + type_size > end_type) {

...specifically the addition of next_type to type_size in the last line above.
A sufficiently high type_size will cause an address wrap on 32-bit systems and
we end up not triggering the bounds check. That seems like an existing issue to
me; even the example cited describes a vlen of 1 with a large elem_sz from the
layout section.

The proposed solution seems to be to check if type_size falls outside of the
remaining space in the types section. It might make sense to do that check locally
in btf_type_size_unknown() instead for clarity and safety. Something like the
following:

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 267904939098..7d0dfb2810e9 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;
        __u32 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)) {
+               pr_debug("Overflow in type size %d for kind %u\n",
+                        type_size, kind);
+               return -EINVAL;
+       }
+       return type_size;
 }

Given that this problem exists separate from the extended vlen series, might make more
sense to send a separate patch targeting the bpf tree for this issue.

Alan