[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+5d1da7...@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 syzk...@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

[ead...@sina.com]

From: eadivs <ead...@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.

Link: https://syzkaller.appspot.com/bug?extid=5d1da78b375c3b5e6c2b
Reported-by: syzbot+5d1da7...@syzkaller.appspotmail.com
Signed-off-by: eadivs <ead...@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

[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+5d1da7...@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin...@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

[s...@google.com]

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+5d1da7...@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>

Thanks!

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

[Stanislav Fomichev]

On Fri, Sep 9, 2022 at 7:07 PM <ead...@sina.com> wrote:
>
> From: eadivs <ead...@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);

?

[Alexei Starovoitov]

On Sat, Sep 10, 2022 at 8:08 AM Tetsuo Handa
<penguin...@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+5d1da7...@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin...@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

[patchwork-b...@kernel.org]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <a...@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+5d1da7...@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin...@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

[ead...@sina.com]

From: Edward Adam Davis <ead...@sina.com>

On Sat, Sep 10, 2022 at 8:29 PM Stanislav Fomichev <s...@google.com> wrote:
> On Fri, Sep 9, 2022 at 7:07 PM <ead...@sina.com> wrote:
> >
> > From: eadivs <ead...@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.

No.
I am using code on mainline, so there are no issues introduced on the
mainline repository, but I found an inappropriate logical use of
HASHTAB_MAP_LOCK_COUNT, so I changed it.
Yes, 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);
>
> ?

Yes, You are right.
I am using the mainline code and I checked the code on both bpf and
bpf-next repositories, commit "86fe28f7692d96d2" exists in the bpf-next
repository, the function htab_map_alloc and the struct bpf_htab are adjusted in
this submission, and it lacked htab->pcount release.