Hello,
syzbot found the following crash on:
HEAD commit: 6d028043 Add linux-next specific files for 20190830
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=135c1a92600000
kernel config: https://syzkaller.appspot.com/x/.config?x=82a6bec43ab0cb69
dashboard link: https://syzkaller.appspot.com/bug?extid=4e7a85b1432052e8d6f8
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=109124e1600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: [email protected]
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 10235 Comm: syz-executor.0 Not tainted 5.3.0-rc6-next-20190830
#75
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:__write_once_size include/linux/compiler.h:203 [inline]
RIP: 0010:__hlist_del include/linux/list.h:795 [inline]
RIP: 0010:hlist_del_rcu include/linux/rculist.h:475 [inline]
RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
Code: 48 89 f1 48 89 75 c8 48 c1 e9 03 80 3c 11 00 0f 85 d3 02 00 00 48 b9
00 00 00 00 00 fc ff df 48 8b 53 10 48 89 d6 48 c1 ee 03 <80> 3c 0e 00 0f
85 97 02 00 00 48 85 c0 48 89 02 74 38 48 89 55 b8
RSP: 0018:ffff88808d607c30 EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffff8880a7f14580 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8880a7f14588
RBP: ffff88808d607c78 R08: 0000000000000004 R09: ffffed1011ac0f73
R10: ffffed1011ac0f72 R11: 0000000000000003 R12: ffff88809f4e9400
R13: ffff88809b06ba00 R14: 0000000000000000 R15: ffff88809f4e9528
FS: 00007f3a3d50c700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007feb3fcd0000 CR3: 00000000986b9000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
map_update_elem+0xc82/0x10b0 kernel/bpf/syscall.c:966
__do_sys_bpf+0x8b5/0x3350 kernel/bpf/syscall.c:2854
__se_sys_bpf kernel/bpf/syscall.c:2825 [inline]
__x64_sys_bpf+0x73/0xb0 kernel/bpf/syscall.c:2825
do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459879
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 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 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f3a3d50bc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000459879
RDX: 0000000000000020 RSI: 0000000020000040 RDI: 0000000000000002
RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3a3d50c6d4
R13: 00000000004bfc86 R14: 00000000004d1960 R15: 00000000ffffffff
Modules linked in:
---[ end trace 083223e21dbd0ae5 ]---
RIP: 0010:__write_once_size include/linux/compiler.h:203 [inline]
RIP: 0010:__hlist_del include/linux/list.h:795 [inline]
RIP: 0010:hlist_del_rcu include/linux/rculist.h:475 [inline]
RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
Code: 48 89 f1 48 89 75 c8 48 c1 e9 03 80 3c 11 00 0f 85 d3 02 00 00 48 b9
00 00 00 00 00 fc ff df 48 8b 53 10 48 89 d6 48 c1 ee 03 <80> 3c 0e 00 0f
85 97 02 00 00 48 85 c0 48 89 02 74 38 48 89 55 b8
RSP: 0018:ffff88808d607c30 EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffff8880a7f14580 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8880a7f14588
RBP: ffff88808d607c78 R08: 0000000000000004 R09: ffffed1011ac0f73
R10: ffffed1011ac0f72 R11: 0000000000000003 R12: ffff88809f4e9400
R13: ffff88809b06ba00 R14: 0000000000000000 R15: ffff88809f4e9528
FS: 00007f3a3d50c700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007feb3fcd0000 CR3: 00000000986b9000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
This bug 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 [email protected].
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
On Thu, Sep 5, 2019 at 1:08 PM syzbot
<[email protected]> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 6d028043 Add linux-next specific files for 20190830
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=135c1a92600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=82a6bec43ab0cb69
> dashboard link: https://syzkaller.appspot.com/bug?extid=4e7a85b1432052e8d6f8
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=109124e1600000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: [email protected]
>
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] PREEMPT SMP KASAN
> CPU: 1 PID: 10235 Comm: syz-executor.0 Not tainted 5.3.0-rc6-next-20190830
> #75
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:__write_once_size include/linux/compiler.h:203 [inline]
> RIP: 0010:__hlist_del include/linux/list.h:795 [inline]
> RIP: 0010:hlist_del_rcu include/linux/rculist.h:475 [inline]
> RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
> RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
> Code: 48 89 f1 48 89 75 c8 48 c1 e9 03 80 3c 11 00 0f 85 d3 02 00 00 48 b9
> 00 00 00 00 00 fc ff df 48 8b 53 10 48 89 d6 48 c1 ee 03 <80> 3c 0e 00 0f
> 85 97 02 00 00 48 85 c0 48 89 02 74 38 48 89 55 b8
> RSP: 0018:ffff88808d607c30 EFLAGS: 00010046
> RAX: 0000000000000000 RBX: ffff8880a7f14580 RCX: dffffc0000000000
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8880a7f14588
> RBP: ffff88808d607c78 R08: 0000000000000004 R09: ffffed1011ac0f73
> R10: ffffed1011ac0f72 R11: 0000000000000003 R12: ffff88809f4e9400
> R13: ffff88809b06ba00 R14: 0000000000000000 R15: ffff88809f4e9528
> FS: 00007f3a3d50c700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007feb3fcd0000 CR3: 00000000986b9000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> map_update_elem+0xc82/0x10b0 kernel/bpf/syscall.c:966
> __do_sys_bpf+0x8b5/0x3350 kernel/bpf/syscall.c:2854
> __se_sys_bpf kernel/bpf/syscall.c:2825 [inline]
> __x64_sys_bpf+0x73/0xb0 kernel/bpf/syscall.c:2825
> do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x459879
> Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 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 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007f3a3d50bc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000459879
> RDX: 0000000000000020 RSI: 0000000020000040 RDI: 0000000000000002
> RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3a3d50c6d4
> R13: 00000000004bfc86 R14: 00000000004d1960 R15: 00000000ffffffff
> Modules linked in:
> ---[ end trace 083223e21dbd0ae5 ]---
> RIP: 0010:__write_once_size include/linux/compiler.h:203 [inline]
> RIP: 0010:__hlist_del include/linux/list.h:795 [inline]
> RIP: 0010:hlist_del_rcu include/linux/rculist.h:475 [inline]
> RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
> RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
Toke,
please take a look.
Thanks!
On Thu, 5 Sep 2019 14:44:37 -0700
Alexei Starovoitov <[email protected]> wrote:
> On Thu, Sep 5, 2019 at 1:08 PM syzbot
> <[email protected]> wrote:
> >
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: 6d028043 Add linux-next specific files for 20190830
> > git tree: linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=135c1a92600000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=82a6bec43ab0cb69
> > dashboard link: https://syzkaller.appspot.com/bug?extid=4e7a85b1432052e8d6f8
> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=109124e1600000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: [email protected]
> >
> > kasan: CONFIG_KASAN_INLINE enabled
> > kasan: GPF could be caused by NULL-ptr deref or user memory access
> > general protection fault: 0000 [#1] PREEMPT SMP KASAN
> > CPU: 1 PID: 10235 Comm: syz-executor.0 Not tainted 5.3.0-rc6-next-20190830
> > #75
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > RIP: 0010:__write_once_size include/linux/compiler.h:203 [inline]
> > RIP: 0010:__hlist_del include/linux/list.h:795 [inline]
> > RIP: 0010:hlist_del_rcu include/linux/rculist.h:475 [inline]
> > RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
> > RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
> > Code: 48 89 f1 48 89 75 c8 48 c1 e9 03 80 3c 11 00 0f 85 d3 02 00 00 48 b9
> > 00 00 00 00 00 fc ff df 48 8b 53 10 48 89 d6 48 c1 ee 03 <80> 3c 0e 00 0f
> > 85 97 02 00 00 48 85 c0 48 89 02 74 38 48 89 55 b8
> > RSP: 0018:ffff88808d607c30 EFLAGS: 00010046
> > RAX: 0000000000000000 RBX: ffff8880a7f14580 RCX: dffffc0000000000
> > RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8880a7f14588
> > RBP: ffff88808d607c78 R08: 0000000000000004 R09: ffffed1011ac0f73
> > R10: ffffed1011ac0f72 R11: 0000000000000003 R12: ffff88809f4e9400
> > R13: ffff88809b06ba00 R14: 0000000000000000 R15: ffff88809f4e9528
> > FS: 00007f3a3d50c700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007feb3fcd0000 CR3: 00000000986b9000 CR4: 00000000001406e0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > Call Trace:
> > map_update_elem+0xc82/0x10b0 kernel/bpf/syscall.c:966
> > __do_sys_bpf+0x8b5/0x3350 kernel/bpf/syscall.c:2854
> > __se_sys_bpf kernel/bpf/syscall.c:2825 [inline]
> > __x64_sys_bpf+0x73/0xb0 kernel/bpf/syscall.c:2825
> > do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0033:0x459879
> > Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 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 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> > RSP: 002b:00007f3a3d50bc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
> > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000459879
> > RDX: 0000000000000020 RSI: 0000000020000040 RDI: 0000000000000002
> > RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3a3d50c6d4
> > R13: 00000000004bfc86 R14: 00000000004d1960 R15: 00000000ffffffff
> > Modules linked in:
> > ---[ end trace 083223e21dbd0ae5 ]---
> > RIP: 0010:__write_once_size include/linux/compiler.h:203 [inline]
> > RIP: 0010:__hlist_del include/linux/list.h:795 [inline]
> > RIP: 0010:hlist_del_rcu include/linux/rculist.h:475 [inline]
> > RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
> > RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
>
> Toke,
> please take a look.
> Thanks!
Hi Toke,
I think the problem is that you read:
old_dev = __dev_map_hash_lookup_elem(map, idx);
Before holding the lock dtab->index_lock...
I'm not sure this is the correct fix, but I think below change should
solve the issue (not even compile tested):
[bpf-next]$ git diff
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 9af048a932b5..c41854a68e9e 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -664,6 +664,9 @@ static int __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
spin_lock_irqsave(&dtab->index_lock, flags);
+ /* Re-read old_dev while holding lock*/
+ old_dev = __dev_map_hash_lookup_elem(map, idx);
+
if (old_dev) {
hlist_del_rcu(&old_dev->index_hlist);
} else {
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
Jesper Dangaard Brouer <[email protected]> writes:
> On Thu, 5 Sep 2019 14:44:37 -0700
> Alexei Starovoitov <[email protected]> wrote:
>
>> On Thu, Sep 5, 2019 at 1:08 PM syzbot
>> <[email protected]> wrote:
>> >
>> > Hello,
>> >
>> > syzbot found the following crash on:
>> >
>> > HEAD commit: 6d028043 Add linux-next specific files for 20190830
>> > git tree: linux-next
>> > console output: https://syzkaller.appspot.com/x/log.txt?x=135c1a92600000
>> > kernel config: https://syzkaller.appspot.com/x/.config?x=82a6bec43ab0cb69
>> > dashboard link: https://syzkaller.appspot.com/bug?extid=4e7a85b1432052e8d6f8
>> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=109124e1600000
>> >
>> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> > Reported-by: [email protected]
>> >
>> > kasan: CONFIG_KASAN_INLINE enabled
>> > kasan: GPF could be caused by NULL-ptr deref or user memory access
>> > general protection fault: 0000 [#1] PREEMPT SMP KASAN
>> > CPU: 1 PID: 10235 Comm: syz-executor.0 Not tainted 5.3.0-rc6-next-20190830
>> > #75
>> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>> > Google 01/01/2011
>> > RIP: 0010:__write_once_size include/linux/compiler.h:203 [inline]
>> > RIP: 0010:__hlist_del include/linux/list.h:795 [inline]
>> > RIP: 0010:hlist_del_rcu include/linux/rculist.h:475 [inline]
>> > RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
>> > RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
>> > Code: 48 89 f1 48 89 75 c8 48 c1 e9 03 80 3c 11 00 0f 85 d3 02 00 00 48 b9
>> > 00 00 00 00 00 fc ff df 48 8b 53 10 48 89 d6 48 c1 ee 03 <80> 3c 0e 00 0f
>> > 85 97 02 00 00 48 85 c0 48 89 02 74 38 48 89 55 b8
>> > RSP: 0018:ffff88808d607c30 EFLAGS: 00010046
>> > RAX: 0000000000000000 RBX: ffff8880a7f14580 RCX: dffffc0000000000
>> > RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8880a7f14588
>> > RBP: ffff88808d607c78 R08: 0000000000000004 R09: ffffed1011ac0f73
>> > R10: ffffed1011ac0f72 R11: 0000000000000003 R12: ffff88809f4e9400
>> > R13: ffff88809b06ba00 R14: 0000000000000000 R15: ffff88809f4e9528
>> > FS: 00007f3a3d50c700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
>> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> > CR2: 00007feb3fcd0000 CR3: 00000000986b9000 CR4: 00000000001406e0
>> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> > Call Trace:
>> > map_update_elem+0xc82/0x10b0 kernel/bpf/syscall.c:966
>> > __do_sys_bpf+0x8b5/0x3350 kernel/bpf/syscall.c:2854
>> > __se_sys_bpf kernel/bpf/syscall.c:2825 [inline]
>> > __x64_sys_bpf+0x73/0xb0 kernel/bpf/syscall.c:2825
>> > do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
>> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
>> > RIP: 0033:0x459879
>> > Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 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 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
>> > RSP: 002b:00007f3a3d50bc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
>> > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000459879
>> > RDX: 0000000000000020 RSI: 0000000020000040 RDI: 0000000000000002
>> > RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
>> > R10: 0000000000000000 R11: 0000000000000246 R12: 00007f3a3d50c6d4
>> > R13: 00000000004bfc86 R14: 00000000004d1960 R15: 00000000ffffffff
>> > Modules linked in:
>> > ---[ end trace 083223e21dbd0ae5 ]---
>> > RIP: 0010:__write_once_size include/linux/compiler.h:203 [inline]
>> > RIP: 0010:__hlist_del include/linux/list.h:795 [inline]
>> > RIP: 0010:hlist_del_rcu include/linux/rculist.h:475 [inline]
>> > RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
>> > RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
>>
>> Toke,
>> please take a look.
>> Thanks!
>
> Hi Toke,
>
> I think the problem is that you read:
> old_dev = __dev_map_hash_lookup_elem(map, idx);
>
> Before holding the lock dtab->index_lock...
>
> I'm not sure this is the correct fix, but I think below change should
> solve the issue (not even compile tested):
>
> [bpf-next]$ git diff
>
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index 9af048a932b5..c41854a68e9e 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -664,6 +664,9 @@ static int __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
>
> spin_lock_irqsave(&dtab->index_lock, flags);
>
> + /* Re-read old_dev while holding lock*/
> + old_dev = __dev_map_hash_lookup_elem(map, idx);
> +
> if (old_dev) {
> hlist_del_rcu(&old_dev->index_hlist);
> } else {
I think you're right that it's a race between reading the old_dev ptr
and the removal, leading to attempts to remove the same element twice.
Your patch would be one way to fix it, another would be to check the
pointer for list poison before removing it. Let me run both approaches
by the bot to make sure it actually fixes the bug; I'll submit a proper
fix that.
-Toke
Hillf Danton <[email protected]> writes:
>> syzbot has found a reproducer for the following crash on Sat, 07 Sep 2019 18:59:06 -0700
>>
>> HEAD commit: a2c11b03 kcm: use BPF_PROG_RUN
>> git tree: bpf-next
>> console output: https://syzkaller.appspot.com/x/log.txt?x=13d46ec1600000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=cf0c85d15c20ade3
>> dashboard link: https://syzkaller.appspot.com/bug?extid=4e7a85b1432052e8d6f8
>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1220b2d1600000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1360b26e600000
>>
>> general protection fault: 0000 [#1] PREEMPT SMP KASAN
>> CPU: 1 PID: 10210 Comm: syz-executor910 Not tainted 5.3.0-rc7+ #0
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>> Google 01/01/2011
>> RIP: 0010:__write_once_size include/linux/compiler.h:226 [inline]
>> RIP: 0010:__hlist_del include/linux/list.h:762 [inline]
>> RIP: 0010:hlist_del_rcu include/linux/rculist.h:455 [inline]
>> RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
>> RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
>
> Fix commit 6f9d451ab1a3 ("xdp: Add devmap_hash map type for looking
> up devices by hashed index")
While this minimal patch does fix the bug (as Jesper already noted), I
prefer to rework the logic instead of just repeating the lookup; a patch
is on its way :)
-Toke
syzbot found a crash in dev_map_hash_update_elem(), when replacing an
element with a new one. Jesper correctly identified the cause of the crash
as a race condition between the initial lookup in the map (which is done
before taking the lock), and the removal of the old element.
Rather than just add a second lookup into the hashmap after taking the
lock, fix this by reworking the function logic to take the lock before the
initial lookup.
Fixes: 6f9d451ab1a3 ("xdp: Add devmap_hash map type for looking up devices by hashed index")
Reported-and-tested-by: [email protected]
Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
---
kernel/bpf/devmap.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 9af048a932b5..d27f3b60ff6d 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -650,19 +650,22 @@ static int __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
u32 ifindex = *(u32 *)value;
u32 idx = *(u32 *)key;
unsigned long flags;
+ int err = -EEXIST;
if (unlikely(map_flags > BPF_EXIST || !ifindex))
return -EINVAL;
+ spin_lock_irqsave(&dtab->index_lock, flags);
+
old_dev = __dev_map_hash_lookup_elem(map, idx);
if (old_dev && (map_flags & BPF_NOEXIST))
- return -EEXIST;
+ goto out_err;
dev = __dev_map_alloc_node(net, dtab, ifindex, idx);
- if (IS_ERR(dev))
- return PTR_ERR(dev);
-
- spin_lock_irqsave(&dtab->index_lock, flags);
+ if (IS_ERR(dev)) {
+ err = PTR_ERR(dev);
+ goto out_err;
+ }
if (old_dev) {
hlist_del_rcu(&old_dev->index_hlist);
@@ -683,6 +686,10 @@ static int __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
call_rcu(&old_dev->rcu, __dev_map_entry_free);
return 0;
+
+out_err:
+ spin_unlock_irqrestore(&dtab->index_lock, flags);
+ return err;
}
static int dev_map_hash_update_elem(struct bpf_map *map, void *key, void *value,
--
2.23.0
syzbot has found a reproducer for the following crash on:
HEAD commit: a2c11b03 kcm: use BPF_PROG_RUN
git tree: bpf-next
console output: https://syzkaller.appspot.com/x/log.txt?x=13d46ec1600000
kernel config: https://syzkaller.appspot.com/x/.config?x=cf0c85d15c20ade3
dashboard link: https://syzkaller.appspot.com/bug?extid=4e7a85b1432052e8d6f8
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1220b2d1600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1360b26e600000
Bisection is inconclusive: the first bad commit could be any of:
116e7dbe Merge branch 'gen-syn-cookie'
91bc3578 selftests/bpf: add test for bpf_tcp_gen_syncookie
637f71c0 selftests/bpf: bpf_tcp_gen_syncookie->bpf_helpers
bf8ff0f8 selftests/bpf: fix clearing buffered output between tests/subtests
3745ee18 bpf: sync bpf.h to tools/
a98bf573 tools: bpftool: add support for reporting the effective cgroup
progs
70d66244 bpf: add bpf_tcp_gen_syncookie helper
9babe825 bpf: always allocate at least 16 bytes for setsockopt hook
9349d600 tcp: add skb-less helpers to retrieve SYN cookie
fd5ef31f selftests/bpf: extend sockopt_sk selftest with TCP_CONGESTION use
case
02bc2b64 Merge branch 'setsockopt-extra-mem'
96511278 tcp: tcp_syn_flood_action read port from socket
a78d0dbe selftests/bpf: add loop test 4
d3406913 Merge branch 'devmap_hash'
1375dc4a tools: Add definitions for devmap_hash map type
8c303960 selftests/bpf: add loop test 5
946152b3 selftests/bpf: test_progs: switch to open_memstream
e4234619 tools/libbpf_probes: Add new devmap_hash type
10fbe211 tools/include/uapi: Add devmap_hash BPF map type
66bd2ec1 selftests/bpf: test_progs: test__printf -> printf
16e910d4 selftests/bpf: test_progs: drop extra trailing tab
6f9d451a xdp: Add devmap_hash map type for looking up devices by hashed
index
682cdbdc Merge branch 'test_progs-stdio'
fca16e51 xdp: Refactor devmap allocation code for reuse
6dbff13c include/bpf.h: Remove map_insert_ctx() stubs
ef20a9b2 libbpf: add helpers for working with BTF types
475e31f8 Merge branch 'revamp-test_progs'
b03bc685 libbpf: convert libbpf code to use new btf helpers
4cedc0da libbpf: add .BTF.ext offset relocation section loading
b207edfe selftests/bpf: convert send_signal.c to use subtests
51436ed7 selftests/bpf: convert bpf_verif_scale.c to sub-tests API
ddc7c304 libbpf: implement BPF CO-RE offset relocation algorithm
2dc26d5a selftests/bpf: add BPF_CORE_READ relocatable read macro
3a516a0a selftests/bpf: add sub-tests support for test_progs
0ff97e56 selftests/bpf: abstract away test log output
df36e621 selftests/bpf: add CO-RE relocs testing setup
002d3afc selftests/bpf: add CO-RE relocs struct flavors tests
329e38f7 selftest/bpf: centralize libbpf logging management for test_progs
e87fd8ba libbpf: return previous print callback from libbpf_set_print
ec6438a9 selftests/bpf: add CO-RE relocs nesting tests
20a9ad2e selftests/bpf: add CO-RE relocs array tests
8160bae2 selftests/bpf: add test selectors by number and name to test_progs
766f2a59 selftests/bpf: revamp test_progs to allow more control
d9db3550 selftests/bpf: add CO-RE relocs enum/ptr/func_proto tests
61098e89 selftests/bpf: prevent headers to be compiled as C code
9654e2ae selftests/bpf: add CO-RE relocs modifiers/typedef tests
943e398d Merge branch 'flow_dissector-input-flags'
d698f9db selftests/bpf: add CO-RE relocs ptr-as-array tests
c1f5e7dd selftests/bpf: add CO-RE relocs ints tests
e853ae77 selftests/bpf: support BPF_FLOW_DISSECTOR_F_STOP_AT_ENCAP
29e1c668 selftests/bpf: add CO-RE relocs misc tests
71c99e32 bpf/flow_dissector: support ipv6 flow_label and
BPF_FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL
726e333f Merge branch 'compile-once-run-everywhere'
ae173a91 selftests/bpf: support BPF_FLOW_DISSECTOR_F_PARSE_1ST_FRAG
57debff2 tools/bpf: sync bpf_flow_keys flags
b7076592 tools/bpf: fix core_reloc.c compilation error
b2ca4e1c bpf/flow_dissector: support flags in BPF_PROG_TEST_RUN
d9973cec xdp: xdp_umem: fix umem pages mapping for 32bits systems
1ac6b126 bpf/flow_dissector: document flags
3783d437 samples/bpf: xdp_fwd rename devmap name to be xdp_tx_ports
086f9568 bpf/flow_dissector: pass input flags to BPF flow dissector program
a32a32cb samples/bpf: make xdp_fwd more practically usable via devmap lookup
03cd1d1a selftests/bpf: Add selftests for bpf_perf_event_output
abcce733 samples/bpf: xdp_fwd explain bpf_fib_lookup return codes
7c4b90d7 bpf: Allow bpf_skb_event_output for a few prog types
9f30cd56 Merge branch 'bpf-xdp-fwd-sample-improvements'
5e31d507 Merge branch 'convert-tests-to-libbpf'
a664a834 tools: bpftool: fix reading from /proc/config.gz
341dfcf8 btf: expose BTF info through sysfs
47da6e4d selftests/bpf: remove perf buffer helpers
c17bec54 samples/bpf: switch trace_output sample to perf_buffer API
d66fa3c7 tools: bpftool: add feature check for zlib
9840a4ff selftests/bpf: fix race in flow dissector tests
f58a4d51 samples/bpf: convert xdp_sample_pkts_user to perf_buffer API
7fd78568 btf: rename /sys/kernel/btf/kernel into /sys/kernel/btf/vmlinux
898ca681 selftests/bpf: switch test_tcpnotify to perf_buffer API
58b80815 selftests/bpf: convert test_get_stack_raw_tp to perf_buffer API
a1916a15 libbpf: attempt to load kernel BTF from sysfs first
72ef80b5 Merge branch 'bpf-libbpf-read-sysfs-btf'
f2a3e4e9 libbpf: provide more helpful message on uninitialized global var
708852dc Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1130846e600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: [email protected]
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 10210 Comm: syz-executor910 Not tainted 5.3.0-rc7+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:__write_once_size include/linux/compiler.h:226 [inline]
RIP: 0010:__hlist_del include/linux/list.h:762 [inline]
RIP: 0010:hlist_del_rcu include/linux/rculist.h:455 [inline]
RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
Code: 48 89 f1 48 89 75 c8 48 c1 e9 03 80 3c 11 00 0f 85 d3 02 00 00 48 b9
00 00 00 00 00 fc ff df 48 8b 53 10 48 89 d6 48 c1 ee 03 <80> 3c 0e 00 0f
85 97 02 00 00 48 85 c0 48 89 02 74 38 48 89 55 b8
RSP: 0018:ffff88808c757c30 EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffff8880a216a980 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8880a216a988
RBP: ffff88808c757c78 R08: 0000000000000004 R09: ffffed10118eaf73
R10: ffffed10118eaf72 R11: 0000000000000003 R12: ffff88808c7fb2c0
R13: ffff88808aa98800 R14: 0000000000000000 R15: ffff88808c7fb3e8
FS: 00007fd4c5528700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fff47596210 CR3: 000000008b442000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
map_update_elem+0xc82/0x10b0 kernel/bpf/syscall.c:966
__do_sys_bpf+0x8b5/0x3350 kernel/bpf/syscall.c:2854
__se_sys_bpf kernel/bpf/syscall.c:2825 [inline]
__x64_sys_bpf+0x73/0xb0 kernel/bpf/syscall.c:2825
do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:296
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x446a29
Code: e8 0c e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 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 0f 83 5b 07 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fd4c5527db8 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
RAX: ffffffffffffffda RBX: 00000000006dbc28 RCX: 0000000000446a29
RDX: 0000000000000020 RSI: 0000000020000180 RDI: 0000000000000002
RBP: 00000000006dbc20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dbc2c
R13: 00007fff4759618f R14: 00007fd4c55289c0 R15: 0000000000000000
Modules linked in:
---[ end trace 9a6d00abce3fe1c8 ]---
RIP: 0010:__write_once_size include/linux/compiler.h:226 [inline]
RIP: 0010:__hlist_del include/linux/list.h:762 [inline]
RIP: 0010:hlist_del_rcu include/linux/rculist.h:455 [inline]
RIP: 0010:__dev_map_hash_update_elem kernel/bpf/devmap.c:668 [inline]
RIP: 0010:dev_map_hash_update_elem+0x3c8/0x6e0 kernel/bpf/devmap.c:691
Code: 48 89 f1 48 89 75 c8 48 c1 e9 03 80 3c 11 00 0f 85 d3 02 00 00 48 b9
00 00 00 00 00 fc ff df 48 8b 53 10 48 89 d6 48 c1 ee 03 <80> 3c 0e 00 0f
85 97 02 00 00 48 85 c0 48 89 02 74 38 48 89 55 b8
RSP: 0018:ffff88808c757c30 EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffff8880a216a980 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8880a216a988
RBP: ffff88808c757c78 R08: 0000000000000004 R09: ffffed10118eaf73
R10: ffffed10118eaf72 R11: 0000000000000003 R12: ffff88808c7fb2c0
R13: ffff88808aa98800 R14: 0000000000000000 R15: ffff88808c7fb3e8
FS: 00007fd4c5528700(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fff47596210 CR3: 000000008b442000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
On Sun, 8 Sep 2019 09:20:16 +0100
Toke Høiland-Jørgensen <[email protected]> wrote:
> syzbot found a crash in dev_map_hash_update_elem(), when replacing an
> element with a new one. Jesper correctly identified the cause of the crash
> as a race condition between the initial lookup in the map (which is done
> before taking the lock), and the removal of the old element.
>
> Rather than just add a second lookup into the hashmap after taking the
> lock, fix this by reworking the function logic to take the lock before the
> initial lookup.
>
> Fixes: 6f9d451ab1a3 ("xdp: Add devmap_hash map type for looking up devices by hashed index")
> Reported-and-tested-by: [email protected]
> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
Acked-by: Jesper Dangaard Brouer <[email protected]>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer