2020-02-26 05:39:30

by syzbot

[permalink] [raw]
Subject: possible deadlock in cma_netdev_callback

Hello,

syzbot found the following crash on:

HEAD commit: 6132c1d9 net: core: devlink.c: Hold devlink->lock from the..
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=16978909e00000
kernel config: https://syzkaller.appspot.com/x/.config?x=3b8906eb6a7d6028
dashboard link: https://syzkaller.appspot.com/bug?extid=55de90ab5f44172b0c90
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12808281e00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=134ca6fde00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: [email protected]

iwpm_register_pid: Unable to send a nlmsg (client = 2)
infiniband syz1: RDMA CMA: cma_listen_on_dev, error -98
netlink: 'syz-executor639': attribute type 1 has an invalid length.
8021q: adding VLAN 0 to HW filter on device bond1
bond1: (slave gretap1): making interface the new active one
======================================================
WARNING: possible circular locking dependency detected
5.6.0-rc2-syzkaller #0 Not tainted
------------------------------------------------------
syz-executor639/9689 is trying to acquire lock:
ffffffff8a5d2a60 (lock#3){+.+.}, at: cma_netdev_callback+0xc6/0x380 drivers/infiniband/core/cma.c:4605

but task is already holding lock:
ffffffff8a74da00 (rtnl_mutex){+.+.}, at: rtnl_lock net/core/rtnetlink.c:72 [inline]
ffffffff8a74da00 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x405/0xaf0 net/core/rtnetlink.c:5433

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (rtnl_mutex){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:956 [inline]
__mutex_lock+0x156/0x13c0 kernel/locking/mutex.c:1103
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1118
rtnl_lock+0x17/0x20 net/core/rtnetlink.c:72
siw_create_listen+0x329/0xed0 drivers/infiniband/sw/siw/siw_cm.c:1951
iw_cm_listen+0x16e/0x1f0 drivers/infiniband/core/iwcm.c:582
cma_iw_listen drivers/infiniband/core/cma.c:2450 [inline]
rdma_listen+0x613/0x970 drivers/infiniband/core/cma.c:3614
cma_listen_on_dev+0x530/0x6a0 drivers/infiniband/core/cma.c:2501
cma_add_one+0x6fe/0xbf0 drivers/infiniband/core/cma.c:4666
add_client_context+0x3dd/0x550 drivers/infiniband/core/device.c:681
enable_device_and_get+0x1df/0x3c0 drivers/infiniband/core/device.c:1316
ib_register_device drivers/infiniband/core/device.c:1382 [inline]
ib_register_device+0xa89/0xe40 drivers/infiniband/core/device.c:1343
siw_device_register drivers/infiniband/sw/siw/siw_main.c:70 [inline]
siw_newlink drivers/infiniband/sw/siw/siw_main.c:565 [inline]
siw_newlink+0xdef/0x1310 drivers/infiniband/sw/siw/siw_main.c:542
nldev_newlink+0x28a/0x430 drivers/infiniband/core/nldev.c:1538
rdma_nl_rcv_msg drivers/infiniband/core/netlink.c:195 [inline]
rdma_nl_rcv_skb drivers/infiniband/core/netlink.c:239 [inline]
rdma_nl_rcv+0x5d9/0x980 drivers/infiniband/core/netlink.c:259
netlink_unicast_kernel net/netlink/af_netlink.c:1303 [inline]
netlink_unicast+0x59e/0x7e0 net/netlink/af_netlink.c:1329
netlink_sendmsg+0x91c/0xea0 net/netlink/af_netlink.c:1918
sock_sendmsg_nosec net/socket.c:652 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:672
____sys_sendmsg+0x753/0x880 net/socket.c:2343
___sys_sendmsg+0x100/0x170 net/socket.c:2397
__sys_sendmsg+0x105/0x1d0 net/socket.c:2430
__do_sys_sendmsg net/socket.c:2439 [inline]
__se_sys_sendmsg net/socket.c:2437 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2437
do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe

-> #0 (lock#3){+.+.}:
check_prev_add kernel/locking/lockdep.c:2475 [inline]
check_prevs_add kernel/locking/lockdep.c:2580 [inline]
validate_chain kernel/locking/lockdep.c:2970 [inline]
__lock_acquire+0x2596/0x4a00 kernel/locking/lockdep.c:3954
lock_acquire+0x190/0x410 kernel/locking/lockdep.c:4484
__mutex_lock_common kernel/locking/mutex.c:956 [inline]
__mutex_lock+0x156/0x13c0 kernel/locking/mutex.c:1103
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1118
cma_netdev_callback+0xc6/0x380 drivers/infiniband/core/cma.c:4605
notifier_call_chain+0xc2/0x230 kernel/notifier.c:83
__raw_notifier_call_chain kernel/notifier.c:361 [inline]
raw_notifier_call_chain+0x2e/0x40 kernel/notifier.c:368
call_netdevice_notifiers_info net/core/dev.c:1948 [inline]
call_netdevice_notifiers_info+0xba/0x130 net/core/dev.c:1933
call_netdevice_notifiers_extack net/core/dev.c:1960 [inline]
call_netdevice_notifiers+0x79/0xa0 net/core/dev.c:1974
bond_change_active_slave+0x185b/0x2050 drivers/net/bonding/bond_main.c:944
bond_select_active_slave+0x276/0xae0 drivers/net/bonding/bond_main.c:986
bond_enslave+0x44ef/0x4af0 drivers/net/bonding/bond_main.c:1823
do_set_master net/core/rtnetlink.c:2468 [inline]
do_set_master+0x1dd/0x240 net/core/rtnetlink.c:2441
__rtnl_newlink+0x13a3/0x1790 net/core/rtnetlink.c:3346
rtnl_newlink+0x69/0xa0 net/core/rtnetlink.c:3377
rtnetlink_rcv_msg+0x45e/0xaf0 net/core/rtnetlink.c:5436
netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2478
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5454
netlink_unicast_kernel net/netlink/af_netlink.c:1303 [inline]
netlink_unicast+0x59e/0x7e0 net/netlink/af_netlink.c:1329
netlink_sendmsg+0x91c/0xea0 net/netlink/af_netlink.c:1918
sock_sendmsg_nosec net/socket.c:652 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:672
____sys_sendmsg+0x753/0x880 net/socket.c:2343
___sys_sendmsg+0x100/0x170 net/socket.c:2397
__sys_sendmsg+0x105/0x1d0 net/socket.c:2430
__do_sys_sendmsg net/socket.c:2439 [inline]
__se_sys_sendmsg net/socket.c:2437 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2437
do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe

other info that might help us debug this:

Possible unsafe locking scenario:

CPU0 CPU1
---- ----
lock(rtnl_mutex);
lock(lock#3);
lock(rtnl_mutex);
lock(lock#3);

*** DEADLOCK ***

1 lock held by syz-executor639/9689:
#0: ffffffff8a74da00 (rtnl_mutex){+.+.}, at: rtnl_lock net/core/rtnetlink.c:72 [inline]
#0: ffffffff8a74da00 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x405/0xaf0 net/core/rtnetlink.c:5433

stack backtrace:
CPU: 0 PID: 9689 Comm: syz-executor639 Not tainted 5.6.0-rc2-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x197/0x210 lib/dump_stack.c:118
print_circular_bug.isra.0.cold+0x163/0x172 kernel/locking/lockdep.c:1684
check_noncircular+0x32e/0x3e0 kernel/locking/lockdep.c:1808
check_prev_add kernel/locking/lockdep.c:2475 [inline]
check_prevs_add kernel/locking/lockdep.c:2580 [inline]
validate_chain kernel/locking/lockdep.c:2970 [inline]
__lock_acquire+0x2596/0x4a00 kernel/locking/lockdep.c:3954
lock_acquire+0x190/0x410 kernel/locking/lockdep.c:4484
__mutex_lock_common kernel/locking/mutex.c:956 [inline]
__mutex_lock+0x156/0x13c0 kernel/locking/mutex.c:1103
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1118
cma_netdev_callback+0xc6/0x380 drivers/infiniband/core/cma.c:4605
notifier_call_chain+0xc2/0x230 kernel/notifier.c:83
__raw_notifier_call_chain kernel/notifier.c:361 [inline]
raw_notifier_call_chain+0x2e/0x40 kernel/notifier.c:368
call_netdevice_notifiers_info net/core/dev.c:1948 [inline]
call_netdevice_notifiers_info+0xba/0x130 net/core/dev.c:1933
call_netdevice_notifiers_extack net/core/dev.c:1960 [inline]
call_netdevice_notifiers+0x79/0xa0 net/core/dev.c:1974
bond_change_active_slave+0x185b/0x2050 drivers/net/bonding/bond_main.c:944
bond_select_active_slave+0x276/0xae0 drivers/net/bonding/bond_main.c:986
bond_enslave+0x44ef/0x4af0 drivers/net/bonding/bond_main.c:1823
do_set_master net/core/rtnetlink.c:2468 [inline]
do_set_master+0x1dd/0x240 net/core/rtnetlink.c:2441
__rtnl_newlink+0x13a3/0x1790 net/core/rtnetlink.c:3346
rtnl_newlink+0x69/0xa0 net/core/rtnetlink.c:3377
rtnetlink_rcv_msg+0x45e/0xaf0 net/core/rtnetlink.c:5436
netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2478
rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5454
netlink_unicast_kernel net/netlink/af_netlink.c:1303 [inline]
netlink_unicast+0x59e/0x7e0 net/netlink/af_netlink.c:1329
netlink_sendmsg+0x91c/0xea0 net/netlink/af_netlink.c:1918
sock_sendmsg_nosec net/socket.c:652 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:672
____sys_sendmsg+0x753/0x880 net/socket.c:2343
___sys_sendmsg+0x100/0x170 net/socket.c:2397
__sys_sendmsg+0x105/0x1d0 net/socket.c:2430
__do_sys_sendmsg net/socket.c:2439 [inline]
__se_sys_sendmsg net/socket.c:2437 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2437
do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x440509
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 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 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fff80af47a8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440509
RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000004
RBP: 00000000006ca018 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000004002c8 R11: 0000000000000246 R12: 0000000000401d90
R13: 0000000000401e20 R14: 0000000000000000 R15: 0000000000000000


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


2020-02-26 20:44:11

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: possible deadlock in cma_netdev_callback

On Tue, Feb 25, 2020 at 09:39:10PM -0800, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 6132c1d9 net: core: devlink.c: Hold devlink->lock from the..
> git tree: net
> console output: https://syzkaller.appspot.com/x/log.txt?x=16978909e00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=3b8906eb6a7d6028
> dashboard link: https://syzkaller.appspot.com/bug?extid=55de90ab5f44172b0c90
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12808281e00000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=134ca6fde00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: [email protected]
>
> iwpm_register_pid: Unable to send a nlmsg (client = 2)
> infiniband syz1: RDMA CMA: cma_listen_on_dev, error -98
> netlink: 'syz-executor639': attribute type 1 has an invalid length.
> 8021q: adding VLAN 0 to HW filter on device bond1
> bond1: (slave gretap1): making interface the new active one
> ======================================================
> WARNING: possible circular locking dependency detected
> 5.6.0-rc2-syzkaller #0 Not tainted
> syz-executor639/9689 is trying to acquire lock:
> ffffffff8a5d2a60 (lock#3){+.+.}, at: cma_netdev_callback+0xc6/0x380 drivers/infiniband/core/cma.c:4605
>
> but task is already holding lock:
> ffffffff8a74da00 (rtnl_mutex){+.+.}, at: rtnl_lock net/core/rtnetlink.c:72 [inline]
> ffffffff8a74da00 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x405/0xaf0 net/core/rtnetlink.c:5433
>

Bernard, this is a siw bug too, it is not allowed to get RTNL in
siw_create_listen() (though this is probably for silly reasons and
could be fixed)

It is not easy to get this into the lockdep, I'll send a different
patch too

Jason

2020-02-27 10:11:41

by Bernard Metzler

[permalink] [raw]
Subject: RE: possible deadlock in cma_netdev_callback

-----"Jason Gunthorpe" <[email protected]> wrote: -----

>To: "syzbot" <[email protected]>
>From: "Jason Gunthorpe" <[email protected]>
>Date: 02/26/2020 09:42PM
>Cc: [email protected], [email protected], [email protected],
>[email protected], [email protected],
>[email protected], [email protected],
>[email protected], [email protected], "Bernard
>Metzler" <[email protected]>
>Subject: [EXTERNAL] Re: possible deadlock in cma_netdev_callback
>
>On Tue, Feb 25, 2020 at 09:39:10PM -0800, syzbot wrote:
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: 6132c1d9 net: core: devlink.c: Hold devlink->lock
>from the..
>> git tree: net
>> console output:
>https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspo
>t.com_x_log.txt-3Fx-3D16978909e00000&d=DwIBAg&c=jf_iaSHvJObTbx-siA1ZO
>g&r=2TaYXQ0T-r8ZO1PP1alNwU_QJcRRLfmYTAgd3QCvqSc&m=I4mBjC4dKAL61lpQH5f
>iT4hLQEibtRif2HwliI2VpTA&s=Pd7_6w9kZzU3DupxBL6qo6piAhk8us2gO-BbCVTDj3
>Q&e=
>> kernel config:
>https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspo
>t.com_x_.config-3Fx-3D3b8906eb6a7d6028&d=DwIBAg&c=jf_iaSHvJObTbx-siA1
>ZOg&r=2TaYXQ0T-r8ZO1PP1alNwU_QJcRRLfmYTAgd3QCvqSc&m=I4mBjC4dKAL61lpQH
>5fiT4hLQEibtRif2HwliI2VpTA&s=qI_ppGZR3Vy01oD9xwfU3or7fBrclf20NYgmTJ0N
>v4k&e=
>> dashboard link:
>https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspo
>t.com_bug-3Fextid-3D55de90ab5f44172b0c90&d=DwIBAg&c=jf_iaSHvJObTbx-si
>A1ZOg&r=2TaYXQ0T-r8ZO1PP1alNwU_QJcRRLfmYTAgd3QCvqSc&m=I4mBjC4dKAL61lp
>QH5fiT4hLQEibtRif2HwliI2VpTA&s=OCNawVVe2X3ySwQUmRx_s2XM3p0r4d4cMFkYU_
>IIAmM&e=
>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>> syz repro:
>https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspo
>t.com_x_repro.syz-3Fx-3D12808281e00000&d=DwIBAg&c=jf_iaSHvJObTbx-siA1
>ZOg&r=2TaYXQ0T-r8ZO1PP1alNwU_QJcRRLfmYTAgd3QCvqSc&m=I4mBjC4dKAL61lpQH
>5fiT4hLQEibtRif2HwliI2VpTA&s=_-Ba4z4VxFdS5ran1HOTqcCl5KtbdPUvvthP_yOT
>bJw&e=
>> C reproducer:
>https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspo
>t.com_x_repro.c-3Fx-3D134ca6fde00000&d=DwIBAg&c=jf_iaSHvJObTbx-siA1ZO
>g&r=2TaYXQ0T-r8ZO1PP1alNwU_QJcRRLfmYTAgd3QCvqSc&m=I4mBjC4dKAL61lpQH5f
>iT4hLQEibtRif2HwliI2VpTA&s=tTrtxQFoWaR89fJY7q7Z2shNBhVzrshezgxE17uS34
>o&e=
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the
>commit:
>> Reported-by: [email protected]
>>
>> iwpm_register_pid: Unable to send a nlmsg (client = 2)
>> infiniband syz1: RDMA CMA: cma_listen_on_dev, error -98
>> netlink: 'syz-executor639': attribute type 1 has an invalid length.
>> 8021q: adding VLAN 0 to HW filter on device bond1
>> bond1: (slave gretap1): making interface the new active one
>> ======================================================
>> WARNING: possible circular locking dependency detected
>> 5.6.0-rc2-syzkaller #0 Not tainted
>> syz-executor639/9689 is trying to acquire lock:
>> ffffffff8a5d2a60 (lock#3){+.+.}, at: cma_netdev_callback+0xc6/0x380
>drivers/infiniband/core/cma.c:4605
>>
>> but task is already holding lock:
>> ffffffff8a74da00 (rtnl_mutex){+.+.}, at: rtnl_lock
>net/core/rtnetlink.c:72 [inline]
>> ffffffff8a74da00 (rtnl_mutex){+.+.}, at:
>rtnetlink_rcv_msg+0x405/0xaf0 net/core/rtnetlink.c:5433
>>
>
>Bernard, this is a siw bug too, it is not allowed to get RTNL in
>siw_create_listen() (though this is probably for silly reasons and
>could be fixed)
>
>It is not easy to get this into the lockdep, I'll send a different
>patch too
>
>Jason
Hi Jason,

Thanks for letting me know! Hmm, we cannot use RCU locks since
we potentially sleep. One solution would be to create a list
of matching interfaces while under lock, unlock and use that
list for calling siw_listen_address() (which may sleep),
right...?

Many thanks,
Bernard.

2020-02-27 15:54:19

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: possible deadlock in cma_netdev_callback

On Thu, Feb 27, 2020 at 10:11:13AM +0000, Bernard Metzler wrote:

> Thanks for letting me know! Hmm, we cannot use RCU locks since
> we potentially sleep. One solution would be to create a list
> of matching interfaces while under lock, unlock and use that
> list for calling siw_listen_address() (which may sleep),
> right...?

Why do you need to iterate over addresses anyhow? Shouldn't the listen
just be done with the address the user gave and a BIND DEVICE to the
device siw is connected to?

Also that loop in siw_create looks wrong to me

Jason

2020-02-27 16:22:05

by Bernard Metzler

[permalink] [raw]
Subject: RE: possible deadlock in cma_netdev_callback

-----"Jason Gunthorpe" <[email protected]> wrote: -----

>To: "Bernard Metzler" <[email protected]>
>From: "Jason Gunthorpe" <[email protected]>
>Date: 02/27/2020 04:53PM
>Cc: "syzbot" <[email protected]>,
>[email protected], [email protected], [email protected],
>[email protected], [email protected],
>[email protected], [email protected],
>[email protected], [email protected]
>Subject: [EXTERNAL] Re: possible deadlock in cma_netdev_callback
>
>On Thu, Feb 27, 2020 at 10:11:13AM +0000, Bernard Metzler wrote:
>
>> Thanks for letting me know! Hmm, we cannot use RCU locks since
>> we potentially sleep. One solution would be to create a list
>> of matching interfaces while under lock, unlock and use that
>> list for calling siw_listen_address() (which may sleep),
>> right...?
>
>Why do you need to iterate over addresses anyhow? Shouldn't the
>listen
>just be done with the address the user gave and a BIND DEVICE to the
>device siw is connected to?

The user may give a wildcard local address, so we'd have
to bind to all addresses of that device...

Best,
Bernard.

>
>Also that loop in siw_create looks wrong to me
>
>Jason
>
>

2020-02-27 16:46:54

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: possible deadlock in cma_netdev_callback

On Thu, Feb 27, 2020 at 04:21:21PM +0000, Bernard Metzler wrote:
>
> >To: "Bernard Metzler" <[email protected]>
> >From: "Jason Gunthorpe" <[email protected]>
> >Date: 02/27/2020 04:53PM
> >Cc: "syzbot" <[email protected]>,
> >[email protected], [email protected], [email protected],
> >[email protected], [email protected],
> >[email protected], [email protected],
> >[email protected], [email protected]
> >Subject: [EXTERNAL] Re: possible deadlock in cma_netdev_callback
> >
> >On Thu, Feb 27, 2020 at 10:11:13AM +0000, Bernard Metzler wrote:
> >
> >> Thanks for letting me know! Hmm, we cannot use RCU locks since
> >> we potentially sleep. One solution would be to create a list
> >> of matching interfaces while under lock, unlock and use that
> >> list for calling siw_listen_address() (which may sleep),
> >> right...?
> >
> >Why do you need to iterate over addresses anyhow? Shouldn't the
> >listen
> >just be done with the address the user gave and a BIND DEVICE to the
> >device siw is connected to?
>
> The user may give a wildcard local address, so we'd have
> to bind to all addresses of that device...

AFAIK a wild card bind using BIND DEVICE works just fine?

Jason

2020-02-28 13:08:15

by Bernard Metzler

[permalink] [raw]
Subject: Re: Re: possible deadlock in cma_netdev_callback

-----"Jason Gunthorpe" <[email protected]> wrote: -----

>To: "Bernard Metzler" <[email protected]>
>From: "Jason Gunthorpe" <[email protected]>
>Date: 02/27/2020 05:46PM
>Cc: "syzbot" <[email protected]>,
>[email protected], [email protected], [email protected],
>[email protected], [email protected],
>[email protected], [email protected],
>[email protected], [email protected]
>Subject: [EXTERNAL] Re: possible deadlock in cma_netdev_callback
>
>On Thu, Feb 27, 2020 at 04:21:21PM +0000, Bernard Metzler wrote:
>>
>> >To: "Bernard Metzler" <[email protected]>
>> >From: "Jason Gunthorpe" <[email protected]>
>> >Date: 02/27/2020 04:53PM
>> >Cc: "syzbot"
><[email protected]>,
>> >[email protected], [email protected], [email protected],
>> >[email protected], [email protected],
>> >[email protected], [email protected],
>> >[email protected], [email protected]
>> >Subject: [EXTERNAL] Re: possible deadlock in cma_netdev_callback
>> >
>> >On Thu, Feb 27, 2020 at 10:11:13AM +0000, Bernard Metzler wrote:
>> >
>> >> Thanks for letting me know! Hmm, we cannot use RCU locks since
>> >> we potentially sleep. One solution would be to create a list
>> >> of matching interfaces while under lock, unlock and use that
>> >> list for calling siw_listen_address() (which may sleep),
>> >> right...?
>> >
>> >Why do you need to iterate over addresses anyhow? Shouldn't the
>> >listen
>> >just be done with the address the user gave and a BIND DEVICE to
>the
>> >device siw is connected to?
>>
>> The user may give a wildcard local address, so we'd have
>> to bind to all addresses of that device...
>
>AFAIK a wild card bind using BIND DEVICE works just fine?
>
>Jason
>
Thanks Jason, absolutely! And it makes things so easy...

Let me prepare and send a patch which drops all that
jumbo mumbo logic to iterate over interface addresses
if the socket interface does the right things anyway.

It implies further simplifications to the siw connection
management, since with that we never have to maintain a
list of listening siw endpoints on a given cm_id; it will
always be max one. I'll cleanup the code accordingly and
prepare an extra cleanup patch, which I can send only
later (have a very tight schedule this week).

Bernard.

2020-02-28 13:35:39

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: Re: possible deadlock in cma_netdev_callback

On Fri, Feb 28, 2020 at 01:05:53PM +0000, Bernard Metzler wrote:
>
> >To: "Bernard Metzler" <[email protected]>
> >From: "Jason Gunthorpe" <[email protected]>
> >Date: 02/27/2020 05:46PM
> >Cc: "syzbot" <[email protected]>,
> >[email protected], [email protected], [email protected],
> >[email protected], [email protected],
> >[email protected], [email protected],
> >[email protected], [email protected]
> >Subject: [EXTERNAL] Re: possible deadlock in cma_netdev_callback
> >
> >On Thu, Feb 27, 2020 at 04:21:21PM +0000, Bernard Metzler wrote:
> >>
> >> >To: "Bernard Metzler" <[email protected]>
> >> >From: "Jason Gunthorpe" <[email protected]>
> >> >Date: 02/27/2020 04:53PM
> >> >Cc: "syzbot"
> ><[email protected]>,
> >> >[email protected], [email protected], [email protected],
> >> >[email protected], [email protected],
> >> >[email protected], [email protected],
> >> >[email protected], [email protected]
> >> >Subject: [EXTERNAL] Re: possible deadlock in cma_netdev_callback
> >> >
> >> >On Thu, Feb 27, 2020 at 10:11:13AM +0000, Bernard Metzler wrote:
> >> >
> >> >> Thanks for letting me know! Hmm, we cannot use RCU locks since
> >> >> we potentially sleep. One solution would be to create a list
> >> >> of matching interfaces while under lock, unlock and use that
> >> >> list for calling siw_listen_address() (which may sleep),
> >> >> right...?
> >> >
> >> >Why do you need to iterate over addresses anyhow? Shouldn't the
> >> >listen
> >> >just be done with the address the user gave and a BIND DEVICE to
> >the
> >> >device siw is connected to?
> >>
> >> The user may give a wildcard local address, so we'd have
> >> to bind to all addresses of that device...
> >
> >AFAIK a wild card bind using BIND DEVICE works just fine?
> >
> >Jason
> >
> Thanks Jason, absolutely! And it makes things so easy...

Probably check to confirm, it just my memory..

Jason

2020-02-28 16:43:51

by Bernard Metzler

[permalink] [raw]
Subject: Re: Re: Re: possible deadlock in cma_netdev_callback

-----"Jason Gunthorpe" <[email protected]> wrote: -----

>To: "Bernard Metzler" <[email protected]>
>From: "Jason Gunthorpe" <[email protected]>
>Date: 02/28/2020 02:35PM
>Cc: "syzbot" <[email protected]>,
>[email protected], [email protected], [email protected],
>[email protected], [email protected],
>[email protected], [email protected],
>[email protected], [email protected]
>Subject: [EXTERNAL] Re: Re: possible deadlock in cma_netdev_callback
>
>On Fri, Feb 28, 2020 at 01:05:53PM +0000, Bernard Metzler wrote:
>>
>> >To: "Bernard Metzler" <[email protected]>
>> >From: "Jason Gunthorpe" <[email protected]>
>> >Date: 02/27/2020 05:46PM
>> >Cc: "syzbot"
><[email protected]>,
>> >[email protected], [email protected], [email protected],
>> >[email protected], [email protected],
>> >[email protected], [email protected],
>> >[email protected], [email protected]
>> >Subject: [EXTERNAL] Re: possible deadlock in cma_netdev_callback
>> >
>> >On Thu, Feb 27, 2020 at 04:21:21PM +0000, Bernard Metzler wrote:
>> >>
>> >> >To: "Bernard Metzler" <[email protected]>
>> >> >From: "Jason Gunthorpe" <[email protected]>
>> >> >Date: 02/27/2020 04:53PM
>> >> >Cc: "syzbot"
>> ><[email protected]>,
>> >> >[email protected], [email protected], [email protected],
>> >> >[email protected], [email protected],
>> >> >[email protected], [email protected],
>> >> >[email protected], [email protected]
>> >> >Subject: [EXTERNAL] Re: possible deadlock in
>cma_netdev_callback
>> >> >
>> >> >On Thu, Feb 27, 2020 at 10:11:13AM +0000, Bernard Metzler
>wrote:
>> >> >
>> >> >> Thanks for letting me know! Hmm, we cannot use RCU locks
>since
>> >> >> we potentially sleep. One solution would be to create a list
>> >> >> of matching interfaces while under lock, unlock and use that
>> >> >> list for calling siw_listen_address() (which may sleep),
>> >> >> right...?
>> >> >
>> >> >Why do you need to iterate over addresses anyhow? Shouldn't the
>> >> >listen
>> >> >just be done with the address the user gave and a BIND DEVICE
>to
>> >the
>> >> >device siw is connected to?
>> >>
>> >> The user may give a wildcard local address, so we'd have
>> >> to bind to all addresses of that device...
>> >
>> >AFAIK a wild card bind using BIND DEVICE works just fine?
>> >
>> >Jason
>> >
>> Thanks Jason, absolutely! And it makes things so easy...
>
>Probably check to confirm, it just my memory..
>
>Jason
>
Well, right, marking a socket via setsockopt SO_BINDTODEVICE
does not work - I get -EPERM. Maybe works only from user land
since the ifname gets copied in from there.

What I tested as working is nailing the scope of wildcard
listen via:
s->sk->sk_bound_dev_if = netdev->ifindex;

Without doing it, wildcard listen would end up covering all
interfaces, even if siw is not attached to some. Also, if siw is
attached to more than one interface, only the first bind call
works of course (for wildcard, the rdma_cm calls me for all
interfaces siw is attached to). So without binding to a
device it is not working.

I am not sure what is the right way of limiting the scope
of a socket to one interface in kernel mode. Is above line
the way to go, or do I miss an interface to do such things?
Anybody could help?

Thanks very much!
Bernard.

2020-02-28 16:51:01

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: Re: Re: possible deadlock in cma_netdev_callback

On Fri, Feb 28, 2020 at 04:42:02PM +0000, Bernard Metzler wrote:

> Well, right, marking a socket via setsockopt SO_BINDTODEVICE
> does not work - I get -EPERM. Maybe works only from user land
> since the ifname gets copied in from there.
>
> What I tested as working is nailing the scope of wildcard
> listen via:
> s->sk->sk_bound_dev_if = netdev->ifindex;

That sounds potentially right

> I am not sure what is the right way of limiting the scope
> of a socket to one interface in kernel mode. Is above line
> the way to go, or do I miss an interface to do such things?
> Anybody could help?

I didn't find an alternative, but not a lot of places touching this
outside the implementators of a socket type.

Jason