Re: [PATCH bpf-next v1 05/14] resolve_btfids: Index BTF ID symbols by address

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] A maliciously crafted or empty symbol size can cause an integer underflow, leading to a 4-billion iteration loop.
- [Low] Calling qsort and bsearch with a NULL base pointer when the array is empty is Undefined Behavior.
--

commit e05f36e9d7ac18c038a5933609937a41496ecf8f
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

resolve_btfids: Index BTF ID symbols by address

This commit introduces an address-sorted index for parsed .BTF_ids symbols.
It uses this index in find_kfunc_flags() to scan BTF_SET8_KFUNCS entries
directly and match each entry back to the requested kfunc.

> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c

[ ... ]

> @@ -480,6 +489,49 @@ static int elf_collect(struct object *obj)
> +static const char *find_name_by_addr(struct object *obj, Elf64_Addr addr)
> +{
> +	struct addr_sym key = { .addr = addr };
> +	struct addr_sym *res;
> +
> +	res = bsearch(&key, obj->addr_syms, obj->nr_addr_syms,
> +		      sizeof(*obj->addr_syms), cmp_addr_sym);

[Severity: Low]
Could this call trigger Undefined Behavior if there are no BTF ID symbols?

If the ELF file lacks these symbols, obj->addr_syms remains NULL and
obj->nr_addr_syms is 0. Calling bsearch with a NULL base pointer is
Undefined Behavior under the C standard, which can cause issues with UBSan
builds.

> +	return res ? res->name : NULL;
> +}

[ ... ]

> @@ -573,8 +625,14 @@ static int symbols_collect(struct object *obj)
>  			return -1;
>  		}
>  		id->addr[id->addr_cnt++] = sym.st_value;
> +
> +		if (push_addr_sym(obj, sym.st_value, id->name))
> +			return -1;
>  	}
>  
> +	qsort(obj->addr_syms, obj->nr_addr_syms, sizeof(*obj->addr_syms),
> +	      cmp_addr_sym);

[Severity: Low]
Does this have the same NULL pointer issue as the bsearch call?

If the array is empty and obj->addr_syms is NULL, calling qsort here
triggers the same Undefined Behavior as described above.

> +
>  	return 0;
>  }

[ ... ]

> @@ -946,43 +1004,41 @@ static int collect_decl_tags(struct btf2btf_context *ctx)
>  static u32 find_kfunc_flags(struct object *obj, struct btf_id *kfunc_id)
>  {
>  	const u32 *elf_data_ptr = obj->efile.idlist->d_buf;
>  	struct btf_id *set_id;
>  	struct rb_node *next;
>  	u64 idx, set_addr;
>  	u32 set_flags;
>  
>  	for (next = rb_first(&obj->sets); next; next = rb_next(next)) {
>  		set_id = rb_entry(next, struct btf_id, rb_node);
>  		if (set_id->kind != BTF_ID_KIND_SET8 || set_id->addr_cnt != 1)
>  			continue;
>  
>  		set_addr = set_id->addr[0];
>  		idx = (set_addr - obj->efile.idlist_addr) / sizeof(u32) + 1;
>  		set_flags = elf_data_ptr[idx];
>  		if (!(set_flags & BTF_SET8_KFUNCS))
>  			continue;
>  
> -		for (u32 i = 0; i < kfunc_id->addr_cnt; i++) {
> -			addr = kfunc_id->addr[i];
> +		for (u32 i = 0; i < set_id->cnt; i++) {

[Severity: Medium]
Could this loop iterate billions of times if a symbol size is unexpectedly
small?

When parsing SET8 symbols, the count is calculated as:

id->cnt = sym.st_size / sizeof(uint64_t) - 1;

If sym.st_size is less than 8, id->cnt (which is a signed int) becomes -1.

Because the loop index 'i' is an unsigned 32-bit integer, the comparison
promotes the signed -1 to 0xFFFFFFFF. Will this cause the tool to hang
spinning through 4 billion iterations if it processes a corrupted or crafted
ELF object?

> +			Elf64_Addr addr = set_addr + sizeof(u64) * (i + 1);
> +			const char *name = find_name_by_addr(obj, addr);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601221805.821394-1-ihor.solodrai@linux.dev?part=5