| Message ID | 42bb08e7d5ec3c1cc5cba43fa1215f80255e99a9.1685367663.git.pabeni@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | ffcd0af9b9169146916aceb4ac5c973ae146c2fc |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [v2,mptcp-net,1/2] mptcp: handle correctly disconnect() failures | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/KVM_Validation__normal__except_selftest_mptcp_join_ | warning | Unstable: 1 failed test(s): packetdrill_fastopen |
| matttbe/KVM_Validation__debug__except_selftest_mptcp_join_ | warning | Unstable: 3 failed test(s): packetdrill_fastopen packetdrill_mp_capable selftest_diag |
| matttbe/KVM_Validation__normal__only_selftest_mptcp_join_ | success | Success! ✅ |
| matttbe/KVM_Validation__debug__only_selftest_mptcp_join_ | success | Success! ✅ |
Hi Paolo, Thank you for your modifications, that's great! But sadly, our CI spotted some issues with it when trying to build it. You can find more details there: https://patchwork.kernel.org/project/mptcp/patch/42bb08e7d5ec3c1cc5cba43fa1215f80255e99a9.1685367663.git.pabeni@redhat.com/ https://github.com/multipath-tcp/mptcp_net-next/actions/runs/5113790043 Status: cancelled Initiator: matttbe Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/2dfdc2932f2e Feel free to reply to this email if you cannot access logs, if you need some support to fix the error, if this doesn't seem to be caused by your modifications or if the error is a false positive one. Cheers, MPTCP GH Action bot Bot operated by Matthieu Baerts (Tessares)
Hi Paolo, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal (except selftest_mptcp_join): - Unstable: 1 failed test(s): packetdrill_fastopen
> On May 29, 2023, at 6:41 AM, Paolo Abeni <pabeni@redhat.com> wrote: > > Christoph reported a devide by zero bug in mptcp_recvmsg(): > > divide error: 0000 [#1] PREEMPT SMP > CPU: 1 PID: 19978 Comm: syz-executor.6 Not tainted 6.4.0-rc2-gffcc7899081b #20 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014 > RIP: 0010:__tcp_select_window+0x30e/0x420 net/ipv4/tcp_output.c:3018 > Code: 11 ff 0f b7 cd c1 e9 0c b8 ff ff ff ff d3 e0 89 c1 f7 d1 01 cb 21 c3 eb 17 e8 2e 83 11 ff 31 db eb 0e e8 25 83 11 ff 89 d8 99 <f7> 7c 24 04 29 d3 65 48 8b 04 25 28 00 00 00 48 3b 44 24 10 75 60 > RSP: 0018:ffffc90000a07a18 EFLAGS: 00010246 > RAX: 000000000000ffd7 RBX: 000000000000ffd7 RCX: 0000000000040000 > RDX: 0000000000000000 RSI: 000000000003ffff RDI: 0000000000040000 > RBP: 000000000000ffd7 R08: ffffffff820cf297 R09: 0000000000000001 > R10: 0000000000000000 R11: ffffffff8103d1a0 R12: 0000000000003f00 > R13: 0000000000300000 R14: ffff888101cf3540 R15: 0000000000180000 > FS: 00007f9af4c09640(0000) GS:ffff88813bd00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000001b33824000 CR3: 000000012f241001 CR4: 0000000000170ee0 > Call Trace: > <TASK> > __tcp_cleanup_rbuf+0x138/0x1d0 net/ipv4/tcp.c:1611 > mptcp_recvmsg+0xcb8/0xdd0 net/mptcp/protocol.c:2034 > inet_recvmsg+0x127/0x1f0 net/ipv4/af_inet.c:861 > ____sys_recvmsg+0x269/0x2b0 net/socket.c:1019 > ___sys_recvmsg+0xe6/0x260 net/socket.c:2764 > do_recvmmsg+0x1a5/0x470 net/socket.c:2858 > __do_sys_recvmmsg net/socket.c:2937 [inline] > __se_sys_recvmmsg net/socket.c:2953 [inline] > __x64_sys_recvmmsg+0xa6/0x130 net/socket.c:2953 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x47/0xa0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x72/0xdc > RIP: 0033:0x7f9af58fc6a9 > Code: 5c c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 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 73 01 c3 48 8b 0d 4f 37 0d 00 f7 d8 64 89 01 48 > RSP: 002b:00007f9af4c08cd8 EFLAGS: 00000246 ORIG_RAX: 000000000000012b > RAX: ffffffffffffffda RBX: 00000000006bc050 RCX: 00007f9af58fc6a9 > RDX: 0000000000000001 RSI: 0000000020000140 RDI: 0000000000000004 > RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000f00 R11: 0000000000000246 R12: 00000000006bc05c > R13: fffffffffffffea8 R14: 00000000006bc050 R15: 000000000001fe40 > </TASK> > > mptcp_recvmsg is allowed to release the msk socket lock when > blocking, and before re-acquiring it another thread could have > switched the sock to TCP_LISTEN status - with a prior > connect(AF_UNSPEC) - also clearing icsk_ack.rcv_mss. > > Address the issue preventing the disconnect if some other process is > concurrently performing a blocking syscall on the same socket, alike > commit XXXXXXXXX ("tcp: deny tcp_disconnect() when threads are waiting"). > > Fixes: a6b118febbab ("mptcp: add receive buffer auto-tuning") > Reported-by: Christoph Paasch <cpaasch@apple.com> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/404 > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > v1 -> v2: > - refactor on top of the TCP fix > note: > - the hash of the TCP fix is missing, such commit should be applied > today or tomorrow morning in the worst-case scenario. > --- > net/mptcp/protocol.c | 6 ++++++ > 1 file changed, 6 insertions(+) Same on this one - it works! Tested-by: Christoph Paasch <cpaasch@apple.com> > > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c > index ce1a70ad37cf..585c84f0b0eb 100644 > --- a/net/mptcp/protocol.c > +++ b/net/mptcp/protocol.c > @@ -3141,6 +3141,12 @@ static int mptcp_disconnect(struct sock *sk, int flags) > { > struct mptcp_sock *msk = mptcp_sk(sk); > > + /* Deny disconnect if other threads are blocked in sk_wait_event() > + * or inet_wait_for_connect(). > + */ > + if (sk->sk_wait_pending) > + return -EBUSY; > + > /* We are on the fastopen error path. We can't call straight into the > * subflows cleanup code due to lock nesting (we are already under > * msk->firstsocket lock). > -- > 2.40.1 > >
Hi Paolo, On 29/05/2023 15:41, Paolo Abeni wrote: > Christoph reported a devide by zero bug in mptcp_recvmsg(): > > divide error: 0000 [#1] PREEMPT SMP > CPU: 1 PID: 19978 Comm: syz-executor.6 Not tainted 6.4.0-rc2-gffcc7899081b #20 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014 > RIP: 0010:__tcp_select_window+0x30e/0x420 net/ipv4/tcp_output.c:3018 > Code: 11 ff 0f b7 cd c1 e9 0c b8 ff ff ff ff d3 e0 89 c1 f7 d1 01 cb 21 c3 eb 17 e8 2e 83 11 ff 31 db eb 0e e8 25 83 11 ff 89 d8 99 <f7> 7c 24 04 29 d3 65 48 8b 04 25 28 00 00 00 48 3b 44 24 10 75 60 > RSP: 0018:ffffc90000a07a18 EFLAGS: 00010246 > RAX: 000000000000ffd7 RBX: 000000000000ffd7 RCX: 0000000000040000 > RDX: 0000000000000000 RSI: 000000000003ffff RDI: 0000000000040000 > RBP: 000000000000ffd7 R08: ffffffff820cf297 R09: 0000000000000001 > R10: 0000000000000000 R11: ffffffff8103d1a0 R12: 0000000000003f00 > R13: 0000000000300000 R14: ffff888101cf3540 R15: 0000000000180000 > FS: 00007f9af4c09640(0000) GS:ffff88813bd00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000001b33824000 CR3: 000000012f241001 CR4: 0000000000170ee0 > Call Trace: > <TASK> > __tcp_cleanup_rbuf+0x138/0x1d0 net/ipv4/tcp.c:1611 > mptcp_recvmsg+0xcb8/0xdd0 net/mptcp/protocol.c:2034 > inet_recvmsg+0x127/0x1f0 net/ipv4/af_inet.c:861 > ____sys_recvmsg+0x269/0x2b0 net/socket.c:1019 > ___sys_recvmsg+0xe6/0x260 net/socket.c:2764 > do_recvmmsg+0x1a5/0x470 net/socket.c:2858 > __do_sys_recvmmsg net/socket.c:2937 [inline] > __se_sys_recvmmsg net/socket.c:2953 [inline] > __x64_sys_recvmmsg+0xa6/0x130 net/socket.c:2953 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x47/0xa0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x72/0xdc > RIP: 0033:0x7f9af58fc6a9 > Code: 5c c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 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 73 01 c3 48 8b 0d 4f 37 0d 00 f7 d8 64 89 01 48 > RSP: 002b:00007f9af4c08cd8 EFLAGS: 00000246 ORIG_RAX: 000000000000012b > RAX: ffffffffffffffda RBX: 00000000006bc050 RCX: 00007f9af58fc6a9 > RDX: 0000000000000001 RSI: 0000000020000140 RDI: 0000000000000004 > RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 > R10: 0000000000000f00 R11: 0000000000000246 R12: 00000000006bc05c > R13: fffffffffffffea8 R14: 00000000006bc050 R15: 000000000001fe40 > </TASK> > > mptcp_recvmsg is allowed to release the msk socket lock when > blocking, and before re-acquiring it another thread could have > switched the sock to TCP_LISTEN status - with a prior > connect(AF_UNSPEC) - also clearing icsk_ack.rcv_mss. > > Address the issue preventing the disconnect if some other process is > concurrently performing a blocking syscall on the same socket, alike > commit XXXXXXXXX ("tcp: deny tcp_disconnect() when threads are waiting"). FYI, I used: 4faeee0cf8a5 > Fixes: a6b118febbab ("mptcp: add receive buffer auto-tuning") > Reported-by: Christoph Paasch <cpaasch@apple.com> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/404 > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > v1 -> v2: > - refactor on top of the TCP fix > note: > - the hash of the TCP fix is missing, such commit should be applied > today or tomorrow morning in the worst-case scenario. Thank you for the patches and the tests! Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net> Now in our tree (fixes for -net) without typos reported by codespell: New patches for t/upstream-net and t/upstream: - 1e8b6536343a: mptcp: handle correctly disconnect() failures - ffcd0af9b916: mptcp: fix possible divide by zero in recvmsg() - Results: cb95946976d7..978898338078 (export-net) - Results: d5bf0f97d0f0..bd369ef919fb (export) Tests are now in progress: https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20230531T155928 https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230531T155928 Cheers, Matt
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index ce1a70ad37cf..585c84f0b0eb 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -3141,6 +3141,12 @@ static int mptcp_disconnect(struct sock *sk, int flags) { struct mptcp_sock *msk = mptcp_sk(sk); + /* Deny disconnect if other threads are blocked in sk_wait_event() + * or inet_wait_for_connect(). + */ + if (sk->sk_wait_pending) + return -EBUSY; + /* We are on the fastopen error path. We can't call straight into the * subflows cleanup code due to lock nesting (we are already under * msk->firstsocket lock).
Christoph reported a devide by zero bug in mptcp_recvmsg(): divide error: 0000 [#1] PREEMPT SMP CPU: 1 PID: 19978 Comm: syz-executor.6 Not tainted 6.4.0-rc2-gffcc7899081b #20 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014 RIP: 0010:__tcp_select_window+0x30e/0x420 net/ipv4/tcp_output.c:3018 Code: 11 ff 0f b7 cd c1 e9 0c b8 ff ff ff ff d3 e0 89 c1 f7 d1 01 cb 21 c3 eb 17 e8 2e 83 11 ff 31 db eb 0e e8 25 83 11 ff 89 d8 99 <f7> 7c 24 04 29 d3 65 48 8b 04 25 28 00 00 00 48 3b 44 24 10 75 60 RSP: 0018:ffffc90000a07a18 EFLAGS: 00010246 RAX: 000000000000ffd7 RBX: 000000000000ffd7 RCX: 0000000000040000 RDX: 0000000000000000 RSI: 000000000003ffff RDI: 0000000000040000 RBP: 000000000000ffd7 R08: ffffffff820cf297 R09: 0000000000000001 R10: 0000000000000000 R11: ffffffff8103d1a0 R12: 0000000000003f00 R13: 0000000000300000 R14: ffff888101cf3540 R15: 0000000000180000 FS: 00007f9af4c09640(0000) GS:ffff88813bd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000001b33824000 CR3: 000000012f241001 CR4: 0000000000170ee0 Call Trace: <TASK> __tcp_cleanup_rbuf+0x138/0x1d0 net/ipv4/tcp.c:1611 mptcp_recvmsg+0xcb8/0xdd0 net/mptcp/protocol.c:2034 inet_recvmsg+0x127/0x1f0 net/ipv4/af_inet.c:861 ____sys_recvmsg+0x269/0x2b0 net/socket.c:1019 ___sys_recvmsg+0xe6/0x260 net/socket.c:2764 do_recvmmsg+0x1a5/0x470 net/socket.c:2858 __do_sys_recvmmsg net/socket.c:2937 [inline] __se_sys_recvmmsg net/socket.c:2953 [inline] __x64_sys_recvmmsg+0xa6/0x130 net/socket.c:2953 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x47/0xa0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x72/0xdc RIP: 0033:0x7f9af58fc6a9 Code: 5c c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 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 73 01 c3 48 8b 0d 4f 37 0d 00 f7 d8 64 89 01 48 RSP: 002b:00007f9af4c08cd8 EFLAGS: 00000246 ORIG_RAX: 000000000000012b RAX: ffffffffffffffda RBX: 00000000006bc050 RCX: 00007f9af58fc6a9 RDX: 0000000000000001 RSI: 0000000020000140 RDI: 0000000000000004 RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000f00 R11: 0000000000000246 R12: 00000000006bc05c R13: fffffffffffffea8 R14: 00000000006bc050 R15: 000000000001fe40 </TASK> mptcp_recvmsg is allowed to release the msk socket lock when blocking, and before re-acquiring it another thread could have switched the sock to TCP_LISTEN status - with a prior connect(AF_UNSPEC) - also clearing icsk_ack.rcv_mss. Address the issue preventing the disconnect if some other process is concurrently performing a blocking syscall on the same socket, alike commit XXXXXXXXX ("tcp: deny tcp_disconnect() when threads are waiting"). Fixes: a6b118febbab ("mptcp: add receive buffer auto-tuning") Reported-by: Christoph Paasch <cpaasch@apple.com> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/404 Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- v1 -> v2: - refactor on top of the TCP fix note: - the hash of the TCP fix is missing, such commit should be applied today or tomorrow morning in the worst-case scenario. --- net/mptcp/protocol.c | 6 ++++++ 1 file changed, 6 insertions(+)