Skip to content

Commit 0b943d9

Browse files
davem330davem330
committed
Merge branch 'mptcp-fix-races-on-accept'
Paolo Abeni says: ==================== mptcp: fix races on accept() This series includes some fixes for accept() races which may cause inconsistent MPTCP socket status and oops. Please see the individual patches for the technical details. ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents bd01942 + fca5c82 commit 0b943d9

File tree

3 files changed

+52
-33
lines changed

3 files changed

+52
-33
lines changed

‎net/mptcp/protocol.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,7 +1332,9 @@ static struct ipv6_pinfo *mptcp_inet6_sk(const struct sock *sk)
13321332
}
13331333
#endif
13341334

1335-
struct sock *mptcp_sk_clone(const struct sock *sk, struct request_sock *req)
1335+
struct sock *mptcp_sk_clone(const struct sock *sk,
1336+
const struct tcp_options_received *opt_rx,
1337+
struct request_sock *req)
13361338
{
13371339
struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
13381340
struct sock *nsk = sk_clone_lock(sk, GFP_ATOMIC);
@@ -1370,14 +1372,15 @@ struct sock *mptcp_sk_clone(const struct sock *sk, struct request_sock *req)
13701372

13711373
msk->write_seq = subflow_req->idsn + 1;
13721374
atomic64_set(&msk->snd_una, msk->write_seq);
1373-
if (subflow_req->remote_key_valid) {
1375+
if (opt_rx->mptcp.mp_capable) {
13741376
msk->can_ack = true;
1375-
msk->remote_key = subflow_req->remote_key;
1377+
msk->remote_key = opt_rx->mptcp.sndr_key;
13761378
mptcp_crypto_key_sha(msk->remote_key, NULL, &ack_seq);
13771379
ack_seq++;
13781380
msk->ack_seq = ack_seq;
13791381
}
13801382

1383+
sock_reset_flag(nsk, SOCK_RCU_FREE);
13811384
/* will be fully established after successful MPC subflow creation */
13821385
inet_sk_state_store(nsk, TCP_SYN_RECV);
13831386
bh_unlock_sock(nsk);
@@ -1779,6 +1782,8 @@ static int mptcp_listen(struct socket *sock, int backlog)
17791782
goto unlock;
17801783
}
17811784

1785+
sock_set_flag(sock->sk, SOCK_RCU_FREE);
1786+
17821787
err = ssock->ops->listen(ssock, backlog);
17831788
inet_sk_state_store(sock->sk, inet_sk_state_load(ssock->sk));
17841789
if (!err)

‎net/mptcp/protocol.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,10 @@ struct mptcp_subflow_request_sock {
206206
struct tcp_request_sock sk;
207207
u16 mp_capable : 1,
208208
mp_join : 1,
209-
backup : 1,
210-
remote_key_valid : 1;
209+
backup : 1;
211210
u8 local_id;
212211
u8 remote_id;
213212
u64 local_key;
214-
u64 remote_key;
215213
u64 idsn;
216214
u32 token;
217215
u32 ssn_offset;
@@ -332,7 +330,9 @@ void mptcp_proto_init(void);
332330
int mptcp_proto_v6_init(void);
333331
#endif
334332

335-
struct sock *mptcp_sk_clone(const struct sock *sk, struct request_sock *req);
333+
struct sock *mptcp_sk_clone(const struct sock *sk,
334+
const struct tcp_options_received *opt_rx,
335+
struct request_sock *req);
336336
void mptcp_get_options(const struct sk_buff *skb,
337337
struct tcp_options_received *opt_rx);
338338

‎net/mptcp/subflow.c

Lines changed: 40 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@ static void subflow_init_req(struct request_sock *req,
133133

134134
subflow_req->mp_capable = 0;
135135
subflow_req->mp_join = 0;
136-
subflow_req->remote_key_valid = 0;
137136

138137
#ifdef CONFIG_TCP_MD5SIG
139138
/* no MPTCP if MD5SIG is enabled on this socket or we may run out of
@@ -376,6 +375,17 @@ static void mptcp_force_close(struct sock *sk)
376375
sk_common_release(sk);
377376
}
378377

378+
static void subflow_ulp_fallback(struct sock *sk,
379+
struct mptcp_subflow_context *old_ctx)
380+
{
381+
struct inet_connection_sock *icsk = inet_csk(sk);
382+
383+
mptcp_subflow_tcp_fallback(sk, old_ctx);
384+
icsk->icsk_ulp_ops = NULL;
385+
rcu_assign_pointer(icsk->icsk_ulp_data, NULL);
386+
tcp_sk(sk)->is_mptcp = 0;
387+
}
388+
379389
static struct sock *subflow_syn_recv_sock(const struct sock *sk,
380390
struct sk_buff *skb,
381391
struct request_sock *req,
@@ -388,10 +398,12 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
388398
struct tcp_options_received opt_rx;
389399
bool fallback_is_fatal = false;
390400
struct sock *new_msk = NULL;
401+
bool fallback = false;
391402
struct sock *child;
392403

393404
pr_debug("listener=%p, req=%p, conn=%p", listener, req, listener->conn);
394405

406+
opt_rx.mptcp.mp_capable = 0;
395407
if (tcp_rsk(req)->is_mptcp == 0)
396408
goto create_child;
397409

@@ -406,20 +418,16 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
406418
goto create_msk;
407419
}
408420

409-
opt_rx.mptcp.mp_capable = 0;
410421
mptcp_get_options(skb, &opt_rx);
411-
if (opt_rx.mptcp.mp_capable) {
412-
subflow_req->remote_key = opt_rx.mptcp.sndr_key;
413-
subflow_req->remote_key_valid = 1;
414-
} else {
415-
subflow_req->mp_capable = 0;
422+
if (!opt_rx.mptcp.mp_capable) {
423+
fallback = true;
416424
goto create_child;
417425
}
418426

419427
create_msk:
420-
new_msk = mptcp_sk_clone(listener->conn, req);
428+
new_msk = mptcp_sk_clone(listener->conn, &opt_rx, req);
421429
if (!new_msk)
422-
subflow_req->mp_capable = 0;
430+
fallback = true;
423431
} else if (subflow_req->mp_join) {
424432
fallback_is_fatal = true;
425433
opt_rx.mptcp.mp_join = 0;
@@ -438,12 +446,18 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
438446
if (child && *own_req) {
439447
struct mptcp_subflow_context *ctx = mptcp_subflow_ctx(child);
440448

441-
/* we have null ctx on TCP fallback, which is fatal on
442-
* MPJ handshake
449+
/* we need to fallback on ctx allocation failure and on pre-reqs
450+
* checking above. In the latter scenario we additionally need
451+
* to reset the context to non MPTCP status.
443452
*/
444-
if (!ctx) {
453+
if (!ctx || fallback) {
445454
if (fallback_is_fatal)
446455
goto close_child;
456+
457+
if (ctx) {
458+
subflow_ulp_fallback(child, ctx);
459+
kfree_rcu(ctx, rcu);
460+
}
447461
goto out;
448462
}
449463

@@ -455,6 +469,13 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
455469
mptcp_pm_new_connection(mptcp_sk(new_msk), 1);
456470
ctx->conn = new_msk;
457471
new_msk = NULL;
472+
473+
/* with OoO packets we can reach here without ingress
474+
* mpc option
475+
*/
476+
ctx->remote_key = opt_rx.mptcp.sndr_key;
477+
ctx->fully_established = opt_rx.mptcp.mp_capable;
478+
ctx->can_ack = opt_rx.mptcp.mp_capable;
458479
} else if (ctx->mp_join) {
459480
struct mptcp_sock *owner;
460481

@@ -474,6 +495,13 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
474495
/* dispose of the left over mptcp master, if any */
475496
if (unlikely(new_msk))
476497
mptcp_force_close(new_msk);
498+
499+
/* check for expected invariant - should never trigger, just help
500+
* catching eariler subtle bugs
501+
*/
502+
WARN_ON_ONCE(*own_req && child && tcp_sk(child)->is_mptcp &&
503+
(!mptcp_subflow_ctx(child) ||
504+
!mptcp_subflow_ctx(child)->conn));
477505
return child;
478506

479507
close_child:
@@ -1076,17 +1104,6 @@ static void subflow_ulp_release(struct sock *sk)
10761104
kfree_rcu(ctx, rcu);
10771105
}
10781106

1079-
static void subflow_ulp_fallback(struct sock *sk,
1080-
struct mptcp_subflow_context *old_ctx)
1081-
{
1082-
struct inet_connection_sock *icsk = inet_csk(sk);
1083-
1084-
mptcp_subflow_tcp_fallback(sk, old_ctx);
1085-
icsk->icsk_ulp_ops = NULL;
1086-
rcu_assign_pointer(icsk->icsk_ulp_data, NULL);
1087-
tcp_sk(sk)->is_mptcp = 0;
1088-
}
1089-
10901107
static void subflow_ulp_clone(const struct request_sock *req,
10911108
struct sock *newsk,
10921109
const gfp_t priority)
@@ -1120,9 +1137,6 @@ static void subflow_ulp_clone(const struct request_sock *req,
11201137
* is fully established only after we receive the remote key
11211138
*/
11221139
new_ctx->mp_capable = 1;
1123-
new_ctx->fully_established = subflow_req->remote_key_valid;
1124-
new_ctx->can_ack = subflow_req->remote_key_valid;
1125-
new_ctx->remote_key = subflow_req->remote_key;
11261140
new_ctx->local_key = subflow_req->local_key;
11271141
new_ctx->token = subflow_req->token;
11281142
new_ctx->ssn_offset = subflow_req->ssn_offset;

0 commit comments

Comments
 (0)