Skip to content

Commit 5943182

Browse files
Paolo Abenijaywang-amazon
authored andcommitted
mptcp: do not queue data on closed subflows
commit c886d70 upstream. Dipanjan reported a syzbot splat at close time: WARNING: CPU: 1 PID: 10818 at net/ipv4/af_inet.c:153 inet_sock_destruct+0x6d0/0x8e0 net/ipv4/af_inet.c:153 Modules linked in: uio_ivshmem(OE) uio(E) CPU: 1 PID: 10818 Comm: kworker/1:16 Tainted: G OE 5.19.0-rc6-g2eae0556bb9d #2 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 Workqueue: events mptcp_worker RIP: 0010:inet_sock_destruct+0x6d0/0x8e0 net/ipv4/af_inet.c:153 Code: 21 02 00 00 41 8b 9c 24 28 02 00 00 e9 07 ff ff ff e8 34 4d 91 f9 89 ee 4c 89 e7 e8 4a 47 60 ff e9 a6 fc ff ff e8 20 4d 91 f9 <0f> 0b e9 84 fe ff ff e8 14 4d 91 f9 0f 0b e9 d4 fd ff ff e8 08 4d RSP: 0018:ffffc9001b35fa78 EFLAGS: 00010246 RAX: 0000000000000000 RBX: 00000000002879d0 RCX: ffff8881326f3b00 RDX: 0000000000000000 RSI: ffff8881326f3b00 RDI: 0000000000000002 RBP: ffff888179662674 R08: ffffffff87e983a0 R09: 0000000000000000 R10: 0000000000000005 R11: 00000000000004ea R12: ffff888179662400 R13: ffff888179662428 R14: 0000000000000001 R15: ffff88817e38e258 FS: 0000000000000000(0000) GS:ffff8881f5f00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000020007bc0 CR3: 0000000179592000 CR4: 0000000000150ee0 Call Trace: <TASK> __sk_destruct+0x4f/0x8e0 net/core/sock.c:2067 sk_destruct+0xbd/0xe0 net/core/sock.c:2112 __sk_free+0xef/0x3d0 net/core/sock.c:2123 sk_free+0x78/0xa0 net/core/sock.c:2134 sock_put include/net/sock.h:1927 [inline] __mptcp_close_ssk+0x50f/0x780 net/mptcp/protocol.c:2351 __mptcp_destroy_sock+0x332/0x760 net/mptcp/protocol.c:2828 mptcp_worker+0x5d2/0xc90 net/mptcp/protocol.c:2586 process_one_work+0x9cc/0x1650 kernel/workqueue.c:2289 worker_thread+0x623/0x1070 kernel/workqueue.c:2436 kthread+0x2e9/0x3a0 kernel/kthread.c:376 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:302 </TASK> The root cause of the problem is that an mptcp-level (re)transmit can race with mptcp_close() and the packet scheduler checks the subflow state before acquiring the socket lock: we can try to (re)transmit on an already closed ssk. Fix the issue checking again the subflow socket status under the subflow socket lock protection. Additionally add the missing check for the fallback-to-tcp case. Fixes: d5f4919 ("mptcp: allow picking different xmit subflows") Reported-by: Dipanjan Das <[email protected]> Reviewed-by: Mat Martineau <[email protected]> Signed-off-by: Paolo Abeni <[email protected]> Signed-off-by: Mat Martineau <[email protected]> Signed-off-by: David S. Miller <[email protected]> [mheyne@: Fixed a couple of conflicts due to some missing commit: - Commit d9ca1de ("mptcp: move page frag allocation in mptcp_sendmsg()") reworked a couple of functions. Before this commit there used to be a check for EAGAIN after mptcp_sendmsg_frag. This is equivalent to the "continue" from this backport and therefore is dropped. Other commits that affected the context: - 5cf92bb ("mptcp: re-enable sndbuf autotune") - b713d00 ("mptcp: really share subflow snd_wnd") - f70cad1 ("mptcp: stop relying on tcp_tx_skb_cache")] Signed-off-by: Maximilian Heyne <[email protected]>
1 parent bf1f603 commit 5943182

File tree

2 files changed

+13
-4
lines changed

2 files changed

+13
-4
lines changed

net/mptcp/protocol.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,9 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
933933
page = dfrag->page;
934934
}
935935

936+
if (unlikely(!__tcp_can_send(ssk)))
937+
return -EAGAIN;
938+
936939
/* compute copy limit */
937940
mss_now = tcp_send_mss(ssk, &size_goal, msg->msg_flags);
938941
*pmss_now = mss_now;
@@ -1107,7 +1110,8 @@ static struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk,
11071110
if (!msk->first)
11081111
return NULL;
11091112
*sndbuf = msk->first->sk_sndbuf;
1110-
return sk_stream_memory_free(msk->first) ? msk->first : NULL;
1113+
return __tcp_can_send(msk->first) &&
1114+
sk_stream_memory_free(msk->first) ? msk->first : NULL;
11111115
}
11121116

11131117
/* re-use last subflow, if the burst allow that */

net/mptcp/protocol.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -380,16 +380,21 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
380380
const struct mptcp_addr_info *remote);
381381
int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock);
382382

383-
static inline bool mptcp_subflow_active(struct mptcp_subflow_context *subflow)
383+
static bool __tcp_can_send(const struct sock *ssk)
384+
{
385+
/* only send if our side has not closed yet */
386+
return ((1 << ssk->sk_state) & (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT));
387+
}
388+
389+
static bool mptcp_subflow_active(struct mptcp_subflow_context *subflow)
384390
{
385391
struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
386392

387393
/* can't send if JOIN hasn't completed yet (i.e. is usable for mptcp) */
388394
if (subflow->request_join && !subflow->fully_established)
389395
return false;
390396

391-
/* only send if our side has not closed yet */
392-
return ((1 << ssk->sk_state) & (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT));
397+
return __tcp_can_send(ssk);
393398
}
394399

395400
static inline void mptcp_subflow_tcp_fallback(struct sock *sk,

0 commit comments

Comments
 (0)