Skip to content

Commit 2c22c06

Browse files
Florian Westphaldavem330
authored andcommitted
mptcp: fix use-after-free on tcp fallback
When an mptcp socket connects to a tcp peer or when a middlebox interferes with tcp options, mptcp needs to fall back to plain tcp. Problem is that mptcp is trying to be too clever in this case: It attempts to close the mptcp meta sk and transparently replace it with the (only) subflow tcp sk. Unfortunately, this is racy -- the socket is already exposed to userspace. Any parallel calls to send/recv/setsockopt etc. can cause use-after-free: BUG: KASAN: use-after-free in atomic_try_cmpxchg include/asm-generic/atomic-instrumented.h:693 [inline] CPU: 1 PID: 2083 Comm: syz-executor.1 Not tainted 5.5.0 #2 atomic_try_cmpxchg include/asm-generic/atomic-instrumented.h:693 [inline] queued_spin_lock include/asm-generic/qspinlock.h:78 [inline] do_raw_spin_lock include/linux/spinlock.h:181 [inline] __raw_spin_lock_bh include/linux/spinlock_api_smp.h:136 [inline] _raw_spin_lock_bh+0x71/0xd0 kernel/locking/spinlock.c:175 spin_lock_bh include/linux/spinlock.h:343 [inline] __lock_sock+0x105/0x190 net/core/sock.c:2414 lock_sock_nested+0x10f/0x140 net/core/sock.c:2938 lock_sock include/net/sock.h:1516 [inline] mptcp_setsockopt+0x2f/0x1f0 net/mptcp/protocol.c:800 __sys_setsockopt+0x152/0x240 net/socket.c:2130 __do_sys_setsockopt net/socket.c:2146 [inline] __se_sys_setsockopt net/socket.c:2143 [inline] __x64_sys_setsockopt+0xba/0x150 net/socket.c:2143 do_syscall_64+0xb7/0x3d0 arch/x86/entry/common.c:294 entry_SYSCALL_64_after_hwframe+0x44/0xa9 While the use-after-free can be resolved, there is another problem: sock->ops and sock->sk assignments are not atomic, i.e. we may get calls into mptcp functions with sock->sk already pointing at the subflow socket, or calls into tcp functions with a mptcp meta sk. Remove the fallback code and call the relevant functions for the (only) subflow in case the mptcp socket is connected to tcp peer. Reported-by: Christoph Paasch <[email protected]> Diagnosed-by: Paolo Abeni <[email protected]> Signed-off-by: Florian Westphal <[email protected]> Reviewed-by: Mat Martineau <[email protected]> Tested-by: Christoph Paasch <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 8b7a07c commit 2c22c06

File tree

1 file changed

+6
-70
lines changed

1 file changed

+6
-70
lines changed

net/mptcp/protocol.c

Lines changed: 6 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -24,58 +24,6 @@
2424

2525
#define MPTCP_SAME_STATE TCP_MAX_STATES
2626

27-
static void __mptcp_close(struct sock *sk, long timeout);
28-
29-
static const struct proto_ops *tcp_proto_ops(struct sock *sk)
30-
{
31-
#if IS_ENABLED(CONFIG_MPTCP_IPV6)
32-
if (sk->sk_family == AF_INET6)
33-
return &inet6_stream_ops;
34-
#endif
35-
return &inet_stream_ops;
36-
}
37-
38-
/* MP_CAPABLE handshake failed, convert msk to plain tcp, replacing
39-
* socket->sk and stream ops and destroying msk
40-
* return the msk socket, as we can't access msk anymore after this function
41-
* completes
42-
* Called with msk lock held, releases such lock before returning
43-
*/
44-
static struct socket *__mptcp_fallback_to_tcp(struct mptcp_sock *msk,
45-
struct sock *ssk)
46-
{
47-
struct mptcp_subflow_context *subflow;
48-
struct socket *sock;
49-
struct sock *sk;
50-
51-
sk = (struct sock *)msk;
52-
sock = sk->sk_socket;
53-
subflow = mptcp_subflow_ctx(ssk);
54-
55-
/* detach the msk socket */
56-
list_del_init(&subflow->node);
57-
sock_orphan(sk);
58-
sock->sk = NULL;
59-
60-
/* socket is now TCP */
61-
lock_sock(ssk);
62-
sock_graft(ssk, sock);
63-
if (subflow->conn) {
64-
/* We can't release the ULP data on a live socket,
65-
* restore the tcp callback
66-
*/
67-
mptcp_subflow_tcp_fallback(ssk, subflow);
68-
sock_put(subflow->conn);
69-
subflow->conn = NULL;
70-
}
71-
release_sock(ssk);
72-
sock->ops = tcp_proto_ops(ssk);
73-
74-
/* destroy the left-over msk sock */
75-
__mptcp_close(sk, 0);
76-
return sock;
77-
}
78-
7927
/* If msk has an initial subflow socket, and the MP_CAPABLE handshake has not
8028
* completed yet or has failed, return the subflow socket.
8129
* Otherwise return NULL.
@@ -93,10 +41,6 @@ static bool __mptcp_needs_tcp_fallback(const struct mptcp_sock *msk)
9341
return msk->first && !sk_is_mptcp(msk->first);
9442
}
9543

96-
/* if the mp_capable handshake has failed, it fallbacks msk to plain TCP,
97-
* releases the socket lock and returns a reference to the now TCP socket.
98-
* Otherwise returns NULL
99-
*/
10044
static struct socket *__mptcp_tcp_fallback(struct mptcp_sock *msk)
10145
{
10246
sock_owned_by_me((const struct sock *)msk);
@@ -105,15 +49,11 @@ static struct socket *__mptcp_tcp_fallback(struct mptcp_sock *msk)
10549
return NULL;
10650

10751
if (msk->subflow) {
108-
/* the first subflow is an active connection, discart the
109-
* paired socket
110-
*/
111-
msk->subflow->sk = NULL;
112-
sock_release(msk->subflow);
113-
msk->subflow = NULL;
52+
release_sock((struct sock *)msk);
53+
return msk->subflow;
11454
}
11555

116-
return __mptcp_fallback_to_tcp(msk, msk->first);
56+
return NULL;
11757
}
11858

11959
static bool __mptcp_can_create_subflow(const struct mptcp_sock *msk)
@@ -640,12 +580,14 @@ static void mptcp_subflow_shutdown(struct sock *ssk, int how)
640580
}
641581

642582
/* Called with msk lock held, releases such lock before returning */
643-
static void __mptcp_close(struct sock *sk, long timeout)
583+
static void mptcp_close(struct sock *sk, long timeout)
644584
{
645585
struct mptcp_subflow_context *subflow, *tmp;
646586
struct mptcp_sock *msk = mptcp_sk(sk);
647587
LIST_HEAD(conn_list);
648588

589+
lock_sock(sk);
590+
649591
mptcp_token_destroy(msk->token);
650592
inet_sk_state_store(sk, TCP_CLOSE);
651593

@@ -662,12 +604,6 @@ static void __mptcp_close(struct sock *sk, long timeout)
662604
sk_common_release(sk);
663605
}
664606

665-
static void mptcp_close(struct sock *sk, long timeout)
666-
{
667-
lock_sock(sk);
668-
__mptcp_close(sk, timeout);
669-
}
670-
671607
static void mptcp_copy_inaddrs(struct sock *msk, const struct sock *ssk)
672608
{
673609
#if IS_ENABLED(CONFIG_MPTCP_IPV6)

0 commit comments

Comments
 (0)