Re: [PATCH bpf-next v1 3/8] resolve_btfids: Keep collected kfuncs in a rbtree

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 7/22/26 4:50 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Silent dismissal of inconsistent kfunc flags during deduplication.
> --
> 
> --- Patch [3]: [PATCH bpf-next v1 3/8] resolve_btfids: Keep collected kfuncs in a rbtree ---
> commit d91e5d0dd32a660902e686ef89cb30a196dda160
> Author: Ihor Solodrai <ihor.solodrai@linux.dev>
> 
> resolve_btfids: Keep collected kfuncs in a rbtree
> 
> This commit changes the storage of collected kfuncs from a dynamically grown
> array to an rbtree keyed by BTF ID. This allows for efficient deduplication
> when kfuncs are declared in multiple sets.
> 
>> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
>> index 3198198b03a78..de4986e1bc3d1 100644
>> --- a/tools/bpf/resolve_btfids/main.c
>> +++ b/tools/bpf/resolve_btfids/main.c
> [ ... ]
>> @@ -979,14 +978,48 @@ static int push_decl_tag_id(struct btf2btf_context *ctx, u32 decl_tag_id)
>>  
>>  static int push_kfunc(struct btf2btf_context *ctx, struct kfunc *kfunc)
>>  {
>> -	if (ensure_mem(&ctx->kfuncs, &ctx->max_kfuncs, ctx->nr_kfuncs + 1))
>> +	struct rb_node **p = &ctx->kfuncs.rb_node;
>> +	struct rb_node *parent = NULL;
>> +	struct kfunc *k;
>> +
>> +	/* Dedup by BTF ID: collecting the same kfunc twice is a no-op. */
>> +	while (*p) {
>> +		parent = *p;
>> +		k = rb_entry(parent, struct kfunc, rb_node);
>> +
>> +		if (kfunc->btf_id < k->btf_id)
>> +			p = &(*p)->rb_left;
>> +		else if (kfunc->btf_id > k->btf_id)
>> +			p = &(*p)->rb_right;
>> +		else
>> +			return 0;
> 
> [Severity: Medium]
> Does this silent return effectively ignore conflicting flags from subsequent
> declarations?

This is deliberate here. Flags are checked for in the patch #8

> 
> If the same kfunc is mistakenly declared in multiple BTF sets with different
> flags (such as KF_ACQUIRE vs none), returning 0 here without checking if
> k->flags == kfunc->flags will silently discard the incoming flags.
> 
> Could this cause critical security flags like KF_ACQUIRE or KF_TRUSTED_ARGS
> to be unexpectedly dropped depending on the processing order?
> 
>> +	}
>> +
>> +	k = zalloc(sizeof(*k));
>> +	if (!k)
>>  		return -ENOMEM;
>