Deadlock in mptcp_sockopt_sync_all()

[darkwrat]

Looks like __mptcp_push_pending can call mptcp_sockopt_sync_all through mptcp_flush_join_list while holding the subflow socket lock. Then, __mptcp_sockopt_sync tries to lock the subflow socket for itself, hence the deadlock.

Could you please advise me on a proper fix if I got the root cause correctly? Should I work on a way to reproduce the issue?

I believe that cork/nodelay patches have provided an easy-ish way for me to hit that by not doing subflow->setsockopt_seq = msk->setsockopt_seq before releasing the subflow socket lock. Does this deserve an improvement?

I don't expect line numbers to match any hash, sorry about that. This is from a cloud VM, where I use rebuilt fedora RPMs with patches applied.

[318974.491932] sysrq: Show Blocked State
[318974.494171] task:ss-server       state:D stack:    0 pid:  938 ppid:     1 flags:0x00000000
[318974.497599] Call Trace:
[318974.498836]  <TASK>
[318974.500244] __schedule (kernel/sched/core.c:4972 kernel/sched/core.c:6253) 
[318974.501917] ? __mod_memcg_state (mm/memcontrol.c:635 mm/memcontrol.c:675 mm/memcontrol.c:669) 
[318974.503458] ? csum_partial (./arch/x86/include/asm/checksum_64.h:171 arch/x86/lib/csum-partial_64.c:136) 
[318974.504894] ? _raw_spin_lock_irqsave (./arch/x86/include/asm/atomic.h:202 ./include/linux/atomic/atomic-instrumented.h:513 ./include/asm-generic/qspinlock.h:82 ./include/linux/spinlock.h:185 ./include/linux/spinlock_api_smp.h:111 kernel/locking/spinlock.c:162) 
[318974.506631] schedule (kernel/sched/core.c:6327 (discriminator 1)) 
[318974.508030] __lock_sock (./include/linux/spinlock.h:354 net/core/sock.c:2747) 
[318974.509664] ? do_wait_intr_irq (kernel/sched/wait.c:408) 
[318974.511439] __lock_sock_fast (./include/linux/spinlock.h:394 net/core/sock.c:3344) 
[318974.513068] mptcp_sockopt_sync_all (net/mptcp/sockopt.c:1247 net/mptcp/sockopt.c:1278) 
[318974.514965] __mptcp_push_pending (net/mptcp/protocol.c:1572) 
[318974.516598] mptcp_sendmsg (net/mptcp/protocol.c:1795) 
[318974.518534] sock_sendmsg (net/socket.c:704 net/socket.c:724) 
[318974.519930] __sys_sendto (net/socket.c:2036) 
[318974.521319] ? do_wait_intr_irq (kernel/sched/wait.c:408) 
[318974.522694] ? fpregs_restore_userregs (arch/x86/kernel/fpu/context.h:60) 
[318974.524201] __x64_sys_sendto (net/socket.c:2048 net/socket.c:2044 net/socket.c:2044) 
[318974.525479] do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80) 
[318974.526565] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:113) 
[318974.528085] RIP: 0033:0x7f9ba546c2d0
[318974.529277] RSP: 002b:00007ffdc3b762d8 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
[318974.531374] RAX: ffffffffffffffda RBX: 00007f9ba56c8060 RCX: 00007f9ba546c2d0
[318974.533451] RDX: 000000000000077a RSI: 0000000000e5e180 RDI: 0000000000000234
[318974.535692] RBP: 0000000000cc57f0 R08: 0000000000000000 R09: 0000000000000000
[318974.537988] R10: 0000000000000000 R11: 0000000000000246 R12: 00007f9ba56c8060
[318974.540142] R13: 0000000000b6ba60 R14: 0000000000cc7840 R15: 41d8685b1d7901b8
[318974.543063]  </TASK>

[fw-strlen]

I'd switch to __mptcp_push_pending() to __mptcp_flush_join_list().

[pabeni]

On Tue, Nov 30, 2021 at 7:36 PM Florian Westphal ***@***.***> wrote: I'd switch to __mptcp_push_pending() to __mptcp_flush_join_list().

Indeed that should work! Probably out of scope here, but I'm wondering if we should try to remove (most) mptcp_flush_join_list() and friends replacing: list_add_tail(&subflow->node, &msk->join_list) with something alike: mptcp_data_lock(); if (!sock_owned_by_user(sk)) { list_add(&subflow->node, &msk->conn_list) sync_socket_options(msk, ssk); } else { list_add_tail(&subflow->node, &msk->join_list) set_bit(MPTCP_FLUSH_JOIN_LIST, &msk->flags); } mptcp_data_unlock(); // .. in mptcp_release_cb() if (test_and_clear_bit(MPTCP_FLUSH_JOIN_LIST, &mptcp_sk(sk)->flags)) mptcp_flush_join_list() WDYT? /P

[darkwrat]

I'd switch to __mptcp_push_pending() to __mptcp_flush_join_list().

Hi Florian, thank you for the suggestion, I've done just that and sent a patch to the list.