* [PATCH bpf-next] bpf: fix btf_task_struct_ids w/o CONFIG_DEBUG_INFO_BTF
@ 2021-11-09 22:24 Song Liu
2021-11-09 22:38 ` Eric Dumazet
0 siblings, 1 reply; 4+ messages in thread
From: Song Liu @ 2021-11-09 22:24 UTC (permalink / raw)
To: bpf, netdev; +Cc: ast, daniel, andrii, kernel-team, Song Liu, Eric Dumazet
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>
Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Song Liu <songliubraving@fb.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
/* BTF ID set registration API for modules */
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] bpf: fix btf_task_struct_ids w/o CONFIG_DEBUG_INFO_BTF
2021-11-09 22:24 [PATCH bpf-next] bpf: fix btf_task_struct_ids w/o CONFIG_DEBUG_INFO_BTF Song Liu
@ 2021-11-09 22:38 ` Eric Dumazet
2021-11-09 22:56 ` Song Liu
0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2021-11-09 22:38 UTC (permalink / raw)
To: Song Liu; +Cc: bpf, netdev, ast, daniel, andrii, Kernel-team
On Tue, Nov 9, 2021 at 2:25 PM Song Liu <songliubraving@fb.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
Also you can add
Reported-by: syzbot+e0d81ec552a21d9071aa@syzkaller.appspotmail.com
> Reported-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Song Liu <songliubraving@fb.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.
>
> /* BTF ID set registration API for modules */
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] bpf: fix btf_task_struct_ids w/o CONFIG_DEBUG_INFO_BTF
2021-11-09 22:38 ` Eric Dumazet
@ 2021-11-09 22:56 ` Song Liu
2021-11-10 0:08 ` Yonghong Song
0 siblings, 1 reply; 4+ messages in thread
From: Song Liu @ 2021-11-09 22:56 UTC (permalink / raw)
To: Eric Dumazet, Yonghong Song; +Cc: bpf, netdev, ast, daniel, andrii, Kernel Team
> On Nov 9, 2021, at 2:38 PM, Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Nov 9, 2021 at 2:25 PM Song Liu <songliubraving@fb.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@syzkaller.appspotmail.com
>
>
>> Reported-by: Eric Dumazet <edumazet@google.com>
>> Signed-off-by: Song Liu <songliubraving@fb.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?
Thanks,
Song
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] bpf: fix btf_task_struct_ids w/o CONFIG_DEBUG_INFO_BTF
2021-11-09 22:56 ` Song Liu
@ 2021-11-10 0:08 ` Yonghong Song
0 siblings, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2021-11-10 0:08 UTC (permalink / raw)
To: Song Liu, Eric Dumazet; +Cc: bpf, netdev, ast, daniel, andrii, Kernel Team
On 11/9/21 2:56 PM, Song Liu wrote:
>
>
>> On Nov 9, 2021, at 2:38 PM, Eric Dumazet <edumazet@google.com> wrote:
>>
>> On Tue, Nov 9, 2021 at 2:25 PM Song Liu <songliubraving@fb.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@syzkaller.appspotmail.com
>>
>>
>>> Reported-by: Eric Dumazet <edumazet@google.com>
>>> Signed-off-by: Song Liu <songliubraving@fb.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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-10 0:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09 22:24 [PATCH bpf-next] bpf: fix btf_task_struct_ids w/o CONFIG_DEBUG_INFO_BTF Song Liu
2021-11-09 22:38 ` Eric Dumazet
2021-11-09 22:56 ` Song Liu
2021-11-10 0:08 ` Yonghong Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).