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

Subsystems: bpf [general] (safe dynamic programs and tools), the rest

STALE1733d

4 messages, 3 authors, 2021-11-10 · open the first message on its own page

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

From: Song Liu <hidden>
Date: 2021-11-09 22:27:06 

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 <redacted>
---
 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

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

From: Eric Dumazet <edumazet@google.com>
Date: 2021-11-09 22:56:02 

On Tue, Nov 9, 2021 at 2:25 PM Song Liu [off-list ref] 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
quoted hunk
Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Song Liu <redacted>
---
 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

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

From: Song Liu <hidden>
Date: 2021-11-09 23:07:35 

On Nov 9, 2021, at 2:38 PM, Eric Dumazet [off-list ref] wrote:

On Tue, Nov 9, 2021 at 2:25 PM Song Liu [off-list ref] wrote:
quoted
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


quoted
Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Song Liu <redacted>
---
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






  

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

    

      From: Yonghong Song <hidden>

      Date: 2021-11-10 00:09:06 
    

  

  

On 11/9/21 2:56 PM, Song Liu wrote:
quoted
On Nov 9, 2021, at 2:38 PM, Eric Dumazet [off-list ref] wrote:

On Tue, Nov 9, 2021 at 2:25 PM Song Liu [off-list ref] wrote:
quoted
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.

quoted
Also you can add

Reported-by: syzbot+e0d81ec552a21d9071aa@syzkaller.appspotmail.com


quoted
Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Song Liu <redacted>
---
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




    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help