linux-kernel - Re: [PATCH bpf-next v4 3/8] resolve_btfids: Introduce enum btf_id

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d9b9e129-349b-4510-bf33-01b831c2174b@linux.dev>
Date: Thu, 18 Dec 2025 09:46:14 -0800
From: Ihor Solodrai <ihor.solodrai@...ux.dev>
To: bot+bpf-ci@...nel.org, alan.maguire@...cle.com, ast@...nel.org,
 arighi@...dia.com, akpm@...ux-foundation.org, andrii@...nel.org,
 morbo@...gle.com, changwoo@...lia.com, daniel@...earbox.net,
 void@...ifault.com, dolinux.peng@...il.com, eddyz87@...il.com,
 haoluo@...gle.com, jolsa@...nel.org, john.fastabend@...il.com,
 corbet@....net, justinstitt@...gle.com, kpsingh@...nel.org,
 martin.lau@...ux.dev, nathan@...nel.org, nick.desaulniers+lkml@...il.com,
 nsc@...nel.org, shuah@...nel.org, song@...nel.org, sdf@...ichev.me,
 tj@...nel.org, yonghong.song@...ux.dev
Cc: bpf@...r.kernel.org, dwarves@...r.kernel.org,
 linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
 sched-ext@...ts.linux.dev, martin.lau@...nel.org, clm@...a.com
Subject: Re: [PATCH bpf-next v4 3/8] resolve_btfids: Introduce enum
 btf_id_kind

On 12/17/25 4:47 PM, bot+bpf-ci@...nel.org wrote:
>> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
>> index b4caae1170dd..da8c7d127632 100644
>> --- a/tools/bpf/resolve_btfids/main.c
>> +++ b/tools/bpf/resolve_btfids/main.c
> 
> [ ... ]
> 
>> @@ -260,22 +279,36 @@ static char *get_id(const char *prefix_end)
>>  	return id;
>>  }
>>
>> -static struct btf_id *add_set(struct object *obj, char *name, bool is_set8)
>> +static struct btf_id *add_set(struct object *obj, char *name, enum btf_id_kind kind)
>>  {
>> +	int len = strlen(name);
>> +	int prefixlen;
>> +	char *id;
>> +
>>  	/*
>>  	 * __BTF_ID__set__name
>>  	 * name =    ^
>>  	 * id   =         ^
>>  	 */
>> -	char *id = name + (is_set8 ? sizeof(BTF_SET8 "__") : sizeof(BTF_SET "__")) - 1;
>> -	int len = strlen(name);
>> +	switch (kind) {
>> +	case BTF_ID_KIND_SET:
>> +		prefixlen = sizeof(BTF_SET "__") - 1;
>> +		break;
>> +	case BTF_ID_KIND_SET8:
>> +		prefixlen = sizeof(BTF_SET8 "__") - 1;
>> +		break;
>> +	default:
>> +		pr_err("Unexpected kind %d passed to %s() for symbol %s\n", kind, __func__, name);
>> +		return NULL;
>> +	}
>> +
>> +	id = name + prefixlen - 1;
> 
> Is this off by one? The original code computed:
> 
>     id = name + sizeof(BTF_SET "__") - 1
> 
> For BTF_SET ("set"), this gives: name + sizeof("set__") - 1 = name + 6 - 1 = name + 5
> 
> The new code computes:
> 
>     prefixlen = sizeof(BTF_SET "__") - 1 = 6 - 1 = 5
>     id = name + prefixlen - 1 = name + 5 - 1 = name + 4
> 
> The -1 appears to be applied twice now, once in the prefixlen assignment and
> again when computing id. Should this line be:
> 
>     id = name + prefixlen;

Yes, this is an off-by-one bug.

Good catch, Opus 4.5. Sonnet 4.5 missed this in v3.

This was "harmless", since the names stored in the trees don't affect
the output.  The comparison between the names still works, as they all
simply got a "_" prefix.  But this only makes the bug sneaky, which is
very bad.

The suggested fix is meh though, IMO a better one is:

diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index 3e88dc862d87..ffc15c249ed4 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -295,10 +295,10 @@ static struct btf_id *add_set(struct object *obj, char *name, enum btf_id_kind k
         */
        switch (kind) {
        case BTF_ID_KIND_SET:
-               prefixlen = sizeof(BTF_SET "__") - 1;
+               prefixlen = sizeof(BTF_SET "__");
                break;
        case BTF_ID_KIND_SET8:
-               prefixlen = sizeof(BTF_SET8 "__") - 1;
+               prefixlen = sizeof(BTF_SET8 "__");
                break;
        default:
                pr_err("Unexpected kind %d passed to %s() for symbol %s\n", kind, __func__, name);

> 
>>  	if (id >= name + len) {
>>  		pr_err("FAILED to parse set name: %s\n", name);
>>  		return NULL;
>>  	}
>>
>> -	return btf_id__add(&obj->sets, id, true);
>> +	return btf_id__add_unique(&obj->sets, id, kind);
>>  }
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20321956014