netdev - Re: [PATCH bpf-next] bpf: fix btf_task_struct_ids w/o CONFIG_DEBUG_INFO

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] [day] [month] [year] [list]

Message-ID: <47eb9d22-23bd-2ff8-0639-ed27574e0bdd@fb.com>
Date:   Tue, 9 Nov 2021 16:08:40 -0800
From:   Yonghong Song <yhs@...com>
To:     Song Liu <songliubraving@...com>,
        Eric Dumazet <edumazet@...gle.com>
CC:     bpf <bpf@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "andrii@...nel.org" <andrii@...nel.org>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH bpf-next] bpf: fix btf_task_struct_ids w/o
 CONFIG_DEBUG_INFO_BTF



On 11/9/21 2:56 PM, Song Liu wrote:
> 
> 
>> On Nov 9, 2021, at 2:38 PM, Eric Dumazet <edumazet@...gle.com> wrote:
>>
>> On Tue, Nov 9, 2021 at 2:25 PM Song Liu <songliubraving@...com> wrote:
>>>
>>> This fixes KASAN oops like
>>>
>>> BUG: KASAN: global-out-of-bounds in task_iter_init+0x212/0x2e7 kernel/bpf/task_iter.c:661
>>> Read of size 4 at addr ffffffff90297404 by task swapper/0/1
>>>
>>> CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.15.0-syzkaller #0
>>> Hardware name: ... Google Compute Engine, BIOS Google 01/01/2011
>>> Call Trace:
>>> <TASK>
>>> __dump_stack lib/dump_stack.c:88 [inline]
>>> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
>>> print_address_description.constprop.0.cold+0xf/0x309 mm/kasan/report.c:256
>>> __kasan_report mm/kasan/report.c:442 [inline]
>>> kasan_report.cold+0x83/0xdf mm/kasan/report.c:459
>>> task_iter_init+0x212/0x2e7 kernel/bpf/task_iter.c:661
>>> do_one_initcall+0x103/0x650 init/main.c:1295
>>> do_initcall_level init/main.c:1368 [inline]
>>> do_initcalls init/main.c:1384 [inline]
>>> do_basic_setup init/main.c:1403 [inline]
>>> kernel_init_freeable+0x6b1/0x73a init/main.c:1606
>>> kernel_init+0x1a/0x1d0 init/main.c:1497
>>> ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295
>>> </TASK>
>>>
>>
>> Please add a Fixes: tag
> 
> Will add it in v2.
> 
>>
>> Also you can add
>>
>> Reported-by: syzbot+e0d81ec552a21d9071aa@...kaller.appspotmail.com
>>
>>
>>> Reported-by: Eric Dumazet <edumazet@...gle.com>
>>> Signed-off-by: Song Liu <songliubraving@...com>
>>> ---
>>> kernel/bpf/btf.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>>> index cdb0fba656006..6db929a5826d4 100644
>>> --- a/kernel/bpf/btf.c
>>> +++ b/kernel/bpf/btf.c
>>> @@ -6342,10 +6342,14 @@ const struct bpf_func_proto bpf_btf_find_by_name_kind_proto = {
>>>         .arg4_type      = ARG_ANYTHING,
>>> };
>>>
>>> +#ifdef CONFIG_DEBUG_INFO_BTF
>>> BTF_ID_LIST_GLOBAL(btf_task_struct_ids)
>>> BTF_ID(struct, task_struct)
>>> BTF_ID(struct, file)
>>> BTF_ID(struct, vm_area_struct)
>>> +#else
>>> +u32 btf_task_struct_ids[3];
>>> +#endif
>>
>> What about adding to  BTF_ID_LIST_GLOBAL() another argument ?
>>
>> BTF_ID_LIST_GLOBAL(btf_task_struct_ids, 3)
>>
>> This would avoid this #ifdef
>>
>> I understand commit 079ef53673f2e3b3ee1728800311f20f28eed4f7
>> hardcoded a [5] value, maybe we can do slightly better with exact
>> boundary checks for KASAN.
> 
> I like this idea.
> 
> Yonghong, I believe you added BTF_ID_LIST_GLOBAL. What do you think
> about this proposal?

I think this is better, so we can have a single place (btf_ids.h) to
have #ifdef CONFIG_DEBUG_INFO_BTF.

> 
> Thanks,
> Song
>