linux-kernel - Re: [PATCH bpf] bpf: add check for invalid name in btf_name_valid

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: <07EBE3E5-61A7-4F64-92BA-24A1DCA9583B@gmail.com>
Date: Fri, 30 Aug 2024 11:03:54 +0900
From: Jeongjun Park <aha310510@...il.com>
To: Eduard Zingerman <eddyz87@...il.com>
Cc: alexei.starovoitov@...il.com, andrii@...nel.org, ast@...nel.org,
 bpf@...r.kernel.org, daniel@...earbox.net, haoluo@...gle.com,
 john.fastabend@...il.com, jolsa@...nel.org, kpsingh@...nel.org,
 linux-kernel@...r.kernel.org, martin.lau@...ux.dev, sdf@...ichev.me,
 song@...nel.org, yonghong.song@...ux.dev
Subject: Re: [PATCH bpf] bpf: add check for invalid name in btf_name_valid_section()



> Eduard Zingerman wrote:
> 
> On Wed, 2024-08-28 at 22:45 -0700, Eduard Zingerman wrote:
> 
> [...]
> 
>> I will prepare a test case.
>> Probably tomorrow.
> 
> Please find test in the attachment. This test triggers KASAN error
> report as in another attachment. (I enabled CONFIG_KASAN using
> menuconfig on top of regular selftest config).
> 

Thank you for writing the selftest for me.

> On Fri, Aug 23, 2024 at 3:43 AM Jeongjun Park <aha310510@...il.com> wrote:
> 
> [...]
> 
>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>> index 520f49f422fe..5c24ea1a65a4 100644
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
>> @@ -823,6 +823,9 @@ static bool btf_name_valid_section(const struct btf *btf, u32 offset)
>>        const char *src = btf_str_by_offset(btf, offset);
>>        const char *src_limit;
>> 
>> +       if (!*src)
>> +               return false;
>> +
> 
> I think that correct fix would be as follows:
> 
> ---
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index edad152cee8e..d583d76fcace 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -820,7 +820,6 @@ static bool btf_name_valid_section(const struct btf *btf, u32 offset)
> 
>        /* set a limit on identifier length */
>        src_limit = src + KSYM_NAME_LEN;
> -       src++;
>        while (*src && src < src_limit) {
>                if (!isprint(*src))
>                        return false;

However, this patch is logically flawed. 
It will return true for invalid names with 
length 1 and src[0] being NULL. So I think 
it's better to stick with the original patch.

> 
> <bad-name-test.patch>
> <bad-name-kasan-report.txt>