[syzbot] WARNING: ODEBUG bug in htab_map_alloc

[syzbot] WARNING: ODEBUG bug in htab_map_alloc

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    274052a2b0ab Merge branch 'bpf-allocator'
git tree:       bpf-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=11a26bcd080000
kernel config:  https://syzkaller.appspot.com/x/.config?x=924833c12349a8c0
dashboard link: https://syzkaller.appspot.com/bug?extid=5d1da78b375c3b5e6c2b
compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=114109f5080000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=11b3b56d080000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/be8eff3df48b/disk-274052a2.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/cd3150e84ddd/vmlinux-274052a2.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+5d1da78b375c3b5e6c2b@syzkaller.appspotmail.com

------------[ cut here ]------------
ODEBUG: free active (active state 0) object type: percpu_counter hint: 0x0
WARNING: CPU: 0 PID: 3624 at lib/debugobjects.c:502 debug_print_object+0x16e/0x250 lib/debugobjects.c:502
Modules linked in:
CPU: 0 PID: 3624 Comm: syz-executor257 Not tainted 5.19.0-syzkaller-14117-g274052a2b0ab #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/26/2022
RIP: 0010:debug_print_object+0x16e/0x250 lib/debugobjects.c:502
Code: ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 af 00 00 00 48 8b 14 dd 60 0c 49 8a 4c 89 ee 48 c7 c7 00 00 49 8a e8 df f1 38 05 <0f> 0b 83 05 65 86 dd 09 01 48 83 c4 18 5b 5d 41 5c 41 5d 41 5e c3
RSP: 0018:ffffc90003edfa90 EFLAGS: 00010282
RAX: 0000000000000000 RBX: 0000000000000003 RCX: 0000000000000000
RDX: ffff8880773cbb00 RSI: ffffffff8161f148 RDI: fffff520007dbf44
RBP: 0000000000000001 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000080000000 R11: 0000000000000000 R12: ffffffff8a4b90c0
R13: ffffffff8a490520 R14: 0000000000000000 R15: dffffc0000000000
FS:  00007f0136485700(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000200004c0 CR3: 0000000072b25000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 __debug_check_no_obj_freed lib/debugobjects.c:989 [inline]
 debug_check_no_obj_freed+0x301/0x420 lib/debugobjects.c:1020
 slab_free_hook mm/slub.c:1729 [inline]
 slab_free_freelist_hook+0xeb/0x1c0 mm/slub.c:1780
 slab_free mm/slub.c:3534 [inline]
 kfree+0xe2/0x580 mm/slub.c:4562
 kvfree+0x42/0x50 mm/util.c:655
 htab_map_alloc+0xc76/0x1620 kernel/bpf/hashtab.c:632
 find_and_alloc_map kernel/bpf/syscall.c:131 [inline]
 map_create kernel/bpf/syscall.c:1105 [inline]
 __sys_bpf+0xa82/0x5f80 kernel/bpf/syscall.c:4938
 __do_sys_bpf kernel/bpf/syscall.c:5060 [inline]
 __se_sys_bpf kernel/bpf/syscall.c:5058 [inline]
 __x64_sys_bpf+0x75/0xb0 kernel/bpf/syscall.c:5058
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7f01364d3919
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 11 15 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f0136485318 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
RAX: ffffffffffffffda RBX: 00007f013655b3e8 RCX: 00007f01364d3919
RDX: 0000000000000048 RSI: 00000000200004c0 RDI: 0000000000000000
RBP: 00007f013655b3e0 R08: 00007f0136485700 R09: 0000000000000000
R10: 00007f0136485700 R11: 0000000000000246 R12: 00007f013655b3ec
R13: 00007ffee9a220af R14: 00007f0136485400 R15: 0000000000022000
 </TASK>
irq event stamp: 19441
hardirqs last  enabled at (19445): [<ffffffff816188e8>] __down_trylock_console_sem+0x108/0x120 kernel/printk/printk.c:247
hardirqs last disabled at (19448): [<ffffffff816188ca>] __down_trylock_console_sem+0xea/0x120 kernel/printk/printk.c:245
softirqs last  enabled at (19350): [<ffffffff814914c3>] invoke_softirq kernel/softirq.c:445 [inline]
softirqs last  enabled at (19350): [<ffffffff814914c3>] __irq_exit_rcu+0x123/0x180 kernel/softirq.c:650
softirqs last disabled at (19341): [<ffffffff814914c3>] invoke_softirq kernel/softirq.c:445 [inline]
softirqs last disabled at (19341): [<ffffffff814914c3>] __irq_exit_rcu+0x123/0x180 kernel/softirq.c:650
---[ end trace 0000000000000000 ]---


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

[PATCH] bpf: add missing percpu_counter_destroy() in htab_map_alloc()

[Tetsuo Handa]

syzbot is reporting ODEBUG bug in htab_map_alloc() [1], for
commit 86fe28f7692d96d2 ("bpf: Optimize element count in non-preallocated
hash map.") added percpu_counter_init() to htab_map_alloc() but forgot to
add percpu_counter_destroy() to the error path.

Link: https://syzkaller.appspot.com/bug?extid=5d1da78b375c3b5e6c2b [1]
Reported-by: syzbot <syzbot+5d1da78b375c3b5e6c2b@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: 86fe28f7692d96d2 ("bpf: Optimize element count in non-preallocated hash map.")
---
 kernel/bpf/hashtab.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 0fe3f136cbbe..86aec20c22d0 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -622,6 +622,8 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
 free_prealloc:
 	prealloc_destroy(htab);
 free_map_locked:
+	if (htab->use_percpu_counter)
+		percpu_counter_destroy(&htab->pcount);
 	for (i = 0; i < HASHTAB_MAP_LOCK_COUNT; i++)
 		free_percpu(htab->map_locked[i]);
 	bpf_map_area_free(htab->buckets);
-- 
2.18.4

Re: [PATCH] bpf: add missing percpu_counter_destroy() in htab_map_alloc()

[sdf]

On 09/11, Tetsuo Handa wrote:
> syzbot is reporting ODEBUG bug in htab_map_alloc() [1], for
> commit 86fe28f7692d96d2 ("bpf: Optimize element count in non-preallocated
> hash map.") added percpu_counter_init() to htab_map_alloc() but forgot to
> add percpu_counter_destroy() to the error path.

> Link: https://syzkaller.appspot.com/bug?extid=5d1da78b375c3b5e6c2b [1]
> Reported-by: syzbot  
> <syzbot+5d1da78b375c3b5e6c2b@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Thanks!

Reviewed-by: Stanislav Fomichev <sdf@google.com>

> Fixes: 86fe28f7692d96d2 ("bpf: Optimize element count in non-preallocated  
> hash map.")
> ---
>   kernel/bpf/hashtab.c | 2 ++
>   1 file changed, 2 insertions(+)

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 0fe3f136cbbe..86aec20c22d0 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -622,6 +622,8 @@ static struct bpf_map *htab_map_alloc(union bpf_attr  
> *attr)
>   free_prealloc:
>   	prealloc_destroy(htab);
>   free_map_locked:
> +	if (htab->use_percpu_counter)
> +		percpu_counter_destroy(&htab->pcount);
>   	for (i = 0; i < HASHTAB_MAP_LOCK_COUNT; i++)
>   		free_percpu(htab->map_locked[i]);
>   	bpf_map_area_free(htab->buckets);
> --
> 2.18.4

Re: [PATCH] bpf: add missing percpu_counter_destroy() in htab_map_alloc()

[Alexei Starovoitov]

On Sat, Sep 10, 2022 at 8:08 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> syzbot is reporting ODEBUG bug in htab_map_alloc() [1], for
> commit 86fe28f7692d96d2 ("bpf: Optimize element count in non-preallocated
> hash map.") added percpu_counter_init() to htab_map_alloc() but forgot to
> add percpu_counter_destroy() to the error path.
>
> Link: https://syzkaller.appspot.com/bug?extid=5d1da78b375c3b5e6c2b [1]
> Reported-by: syzbot <syzbot+5d1da78b375c3b5e6c2b@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Fixes: 86fe28f7692d96d2 ("bpf: Optimize element count in non-preallocated hash map.")
> ---
>  kernel/bpf/hashtab.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 0fe3f136cbbe..86aec20c22d0 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -622,6 +622,8 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
>  free_prealloc:
>         prealloc_destroy(htab);
>  free_map_locked:
> +       if (htab->use_percpu_counter)
> +               percpu_counter_destroy(&htab->pcount);

Thank you for the fix! Applied

Re: [PATCH] bpf: add missing percpu_counter_destroy() in htab_map_alloc()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Sun, 11 Sep 2022 00:07:11 +0900 you wrote:
> syzbot is reporting ODEBUG bug in htab_map_alloc() [1], for
> commit 86fe28f7692d96d2 ("bpf: Optimize element count in non-preallocated
> hash map.") added percpu_counter_init() to htab_map_alloc() but forgot to
> add percpu_counter_destroy() to the error path.
> 
> Link: https://syzkaller.appspot.com/bug?extid=5d1da78b375c3b5e6c2b [1]
> Reported-by: syzbot <syzbot+5d1da78b375c3b5e6c2b@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Fixes: 86fe28f7692d96d2 ("bpf: Optimize element count in non-preallocated hash map.")
> 
> [...]

Here is the summary with links:
  - bpf: add missing percpu_counter_destroy() in htab_map_alloc()
    https://git.kernel.org/bpf/bpf-next/c/cf7de6a53600

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] kernel/bpf: htab_map_alloc() exit by free_map_locked logic issue

[Stanislav Fomichev]

On Fri, Sep 9, 2022 at 7:07 PM <eadavis@sina.com> wrote:
>
> From: eadivs <eadivs@sina.com>
>
> syzbot is reporting WARNING: ODEBUG bug in htab_map_alloc(), the
> loop exits without reaching length HASHTAB_MAP_LOCK_COUNT, and
> the loop continues HASHTAB_MAP_LOCK_COUNT times in label
> free_map_locked.

Please use [PATCH bpf] vs [PATCH bpf-next] in subject to indicate
which tree you're targeting.
Also, it seems your email hasn't reached the mailing list for some reason.

Are you sure that the issue is due to HASHTAB_MAP_LOCK_COUNT? The code
seems fine as is; unconditionally calling free on NULL shouldn't be an
issue.

 htab_map_alloc+0xc76/0x1620 kernel/bpf/hashtab.c:632

Which, if I'm looking at the function is:
bpf_map_area_free(htab);

?

> Link: https://syzkaller.appspot.com/bug?extid=5d1da78b375c3b5e6c2b
> Reported-by: syzbot+5d1da78b375c3b5e6c2b@syzkaller.appspotmail.com
> Signed-off-by: eadivs <eadivs@sina.com>
> ---
>  kernel/bpf/hashtab.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 65877967f414..f5381e1c00a6 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -473,7 +473,7 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
>         bool percpu_lru = (attr->map_flags & BPF_F_NO_COMMON_LRU);
>         bool prealloc = !(attr->map_flags & BPF_F_NO_PREALLOC);
>         struct bpf_htab *htab;
> -       int err, i;
> +       int err, i, j = HASHTAB_MAP_LOCK_COUNT;
>
>         htab = kzalloc(sizeof(*htab), GFP_USER | __GFP_ACCOUNT);
>         if (!htab)
> @@ -523,8 +523,10 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
>                                                            sizeof(int),
>                                                            sizeof(int),
>                                                            GFP_USER);
> -               if (!htab->map_locked[i])
> +               if (!htab->map_locked[i]) {
> +                       j = i;
>                         goto free_map_locked;
> +               }
>         }
>
>         if (htab->map.map_flags & BPF_F_ZERO_SEED)
> @@ -554,7 +556,7 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
>  free_prealloc:
>         prealloc_destroy(htab);
>  free_map_locked:
> -       for (i = 0; i < HASHTAB_MAP_LOCK_COUNT; i++)
> +       for (i = 0; i < j; i++)
>                 free_percpu(htab->map_locked[i]);
>         bpf_map_area_free(htab->buckets);
>  free_htab:
> --
> 2.37.2
>