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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On Nov 9, 2021, at 2:38 PM, Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
> 
> On Tue, Nov 9, 2021 at 2:25 PM Song Liu <songliubraving@xxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> 
> 
>> Reported-by: Eric Dumazet <edumazet@xxxxxxxxxx>
>> Signed-off-by: Song Liu <songliubraving@xxxxxx>
>> ---
>> 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?

Thanks,
Song




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux