Skip to content

Commit 54999de

Browse files
Paolo Abenigregkh
authored andcommitted
mptcp: make fallback action and fallback decision atomic
commit f8a1d9b upstream. Syzkaller reported the following splat: WARNING: CPU: 1 PID: 7704 at net/mptcp/protocol.h:1223 __mptcp_do_fallback net/mptcp/protocol.h:1223 [inline] WARNING: CPU: 1 PID: 7704 at net/mptcp/protocol.h:1223 mptcp_do_fallback net/mptcp/protocol.h:1244 [inline] WARNING: CPU: 1 PID: 7704 at net/mptcp/protocol.h:1223 check_fully_established net/mptcp/options.c:982 [inline] WARNING: CPU: 1 PID: 7704 at net/mptcp/protocol.h:1223 mptcp_incoming_options+0x21a8/0x2510 net/mptcp/options.c:1153 Modules linked in: CPU: 1 UID: 0 PID: 7704 Comm: syz.3.1419 Not tainted 6.16.0-rc3-gbd5ce2324dba #20 PREEMPT(voluntary) Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 RIP: 0010:__mptcp_do_fallback net/mptcp/protocol.h:1223 [inline] RIP: 0010:mptcp_do_fallback net/mptcp/protocol.h:1244 [inline] RIP: 0010:check_fully_established net/mptcp/options.c:982 [inline] RIP: 0010:mptcp_incoming_options+0x21a8/0x2510 net/mptcp/options.c:1153 Code: 24 18 e8 bb 2a 00 fd e9 1b df ff ff e8 b1 21 0f 00 e8 ec 5f c4 fc 44 0f b7 ac 24 b0 00 00 00 e9 54 f1 ff ff e8 d9 5f c4 fc 90 <0f> 0b 90 e9 b8 f4 ff ff e8 8b 2a 00 fd e9 8d e6 ff ff e8 81 2a 00 RSP: 0018:ffff8880a3f08448 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff8880180a8000 RCX: ffffffff84afcf45 RDX: ffff888090223700 RSI: ffffffff84afdaa7 RDI: 0000000000000001 RBP: ffff888017955780 R08: 0000000000000001 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 R13: ffff8880180a8910 R14: ffff8880a3e9d058 R15: 0000000000000000 FS: 00005555791b8500(0000) GS:ffff88811c495000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 000000110c2800b7 CR3: 0000000058e44000 CR4: 0000000000350ef0 Call Trace: <IRQ> tcp_reset+0x26f/0x2b0 net/ipv4/tcp_input.c:4432 tcp_validate_incoming+0x1057/0x1b60 net/ipv4/tcp_input.c:5975 tcp_rcv_established+0x5b5/0x21f0 net/ipv4/tcp_input.c:6166 tcp_v4_do_rcv+0x5dc/0xa70 net/ipv4/tcp_ipv4.c:1925 tcp_v4_rcv+0x3473/0x44a0 net/ipv4/tcp_ipv4.c:2363 ip_protocol_deliver_rcu+0xba/0x480 net/ipv4/ip_input.c:205 ip_local_deliver_finish+0x2f1/0x500 net/ipv4/ip_input.c:233 NF_HOOK include/linux/netfilter.h:317 [inline] NF_HOOK include/linux/netfilter.h:311 [inline] ip_local_deliver+0x1be/0x560 net/ipv4/ip_input.c:254 dst_input include/net/dst.h:469 [inline] ip_rcv_finish net/ipv4/ip_input.c:447 [inline] NF_HOOK include/linux/netfilter.h:317 [inline] NF_HOOK include/linux/netfilter.h:311 [inline] ip_rcv+0x514/0x810 net/ipv4/ip_input.c:567 __netif_receive_skb_one_core+0x197/0x1e0 net/core/dev.c:5975 __netif_receive_skb+0x1f/0x120 net/core/dev.c:6088 process_backlog+0x301/0x1360 net/core/dev.c:6440 __napi_poll.constprop.0+0xba/0x550 net/core/dev.c:7453 napi_poll net/core/dev.c:7517 [inline] net_rx_action+0xb44/0x1010 net/core/dev.c:7644 handle_softirqs+0x1d0/0x770 kernel/softirq.c:579 do_softirq+0x3f/0x90 kernel/softirq.c:480 </IRQ> <TASK> __local_bh_enable_ip+0xed/0x110 kernel/softirq.c:407 local_bh_enable include/linux/bottom_half.h:33 [inline] inet_csk_listen_stop+0x2c5/0x1070 net/ipv4/inet_connection_sock.c:1524 mptcp_check_listen_stop.part.0+0x1cc/0x220 net/mptcp/protocol.c:2985 mptcp_check_listen_stop net/mptcp/mib.h:118 [inline] __mptcp_close+0x9b9/0xbd0 net/mptcp/protocol.c:3000 mptcp_close+0x2f/0x140 net/mptcp/protocol.c:3066 inet_release+0xed/0x200 net/ipv4/af_inet.c:435 inet6_release+0x4f/0x70 net/ipv6/af_inet6.c:487 __sock_release+0xb3/0x270 net/socket.c:649 sock_close+0x1c/0x30 net/socket.c:1439 __fput+0x402/0xb70 fs/file_table.c:465 task_work_run+0x150/0x240 kernel/task_work.c:227 resume_user_mode_work include/linux/resume_user_mode.h:50 [inline] exit_to_user_mode_loop+0xd4/0xe0 kernel/entry/common.c:114 exit_to_user_mode_prepare include/linux/entry-common.h:330 [inline] syscall_exit_to_user_mode_work include/linux/entry-common.h:414 [inline] syscall_exit_to_user_mode include/linux/entry-common.h:449 [inline] do_syscall_64+0x245/0x360 arch/x86/entry/syscall_64.c:100 entry_SYSCALL_64_after_hwframe+0x77/0x7f RIP: 0033:0x7fc92f8a36ad Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 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 c7 c1 b0 ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007ffcf52802d8 EFLAGS: 00000246 ORIG_RAX: 00000000000001b4 RAX: 0000000000000000 RBX: 00007ffcf52803a8 RCX: 00007fc92f8a36ad RDX: 0000000000000000 RSI: 000000000000001e RDI: 0000000000000003 RBP: 00007fc92fae7ba0 R08: 0000000000000001 R09: 0000002800000000 R10: 00007fc92f700000 R11: 0000000000000246 R12: 00007fc92fae5fac R13: 00007fc92fae5fa0 R14: 0000000000026d00 R15: 0000000000026c51 </TASK> irq event stamp: 4068 hardirqs last enabled at (4076): [<ffffffff81544816>] __up_console_sem+0x76/0x80 kernel/printk/printk.c:344 hardirqs last disabled at (4085): [<ffffffff815447fb>] __up_console_sem+0x5b/0x80 kernel/printk/printk.c:342 softirqs last enabled at (3096): [<ffffffff840e1be0>] local_bh_enable include/linux/bottom_half.h:33 [inline] softirqs last enabled at (3096): [<ffffffff840e1be0>] inet_csk_listen_stop+0x2c0/0x1070 net/ipv4/inet_connection_sock.c:1524 softirqs last disabled at (3097): [<ffffffff813b6b9f>] do_softirq+0x3f/0x90 kernel/softirq.c:480 Since we need to track the 'fallback is possible' condition and the fallback status separately, there are a few possible races open between the check and the actual fallback action. Add a spinlock to protect the fallback related information and use it close all the possible related races. While at it also remove the too-early clearing of allow_infinite_fallback in __mptcp_subflow_connect(): the field will be correctly cleared by subflow_finish_connect() if/when the connection will complete successfully. If fallback is not possible, as per RFC, reset the current subflow. Since the fallback operation can now fail and return value should be checked, rename the helper accordingly. Fixes: 0530020 ("mptcp: track and update contiguous data status") Cc: stable@vger.kernel.org Reported-by: Matthieu Baerts <matttbe@kernel.org> Closes: multipath-tcp/mptcp_net-next#570 Reported-by: syzbot+5cf807c20386d699b524@syzkaller.appspotmail.com Closes: multipath-tcp/mptcp_net-next#555 Signed-off-by: Paolo Abeni <pabeni@redhat.com> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> Link: https://patch.msgid.link/20250714-net-mptcp-fallback-races-v1-1-391aff963322@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent f28044a commit 54999de

File tree

4 files changed

+61
-19
lines changed

4 files changed

+61
-19
lines changed

net/mptcp/options.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -979,8 +979,9 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
979979
if (subflow->mp_join)
980980
goto reset;
981981
subflow->mp_capable = 0;
982+
if (!mptcp_try_fallback(ssk))
983+
goto reset;
982984
pr_fallback(msk);
983-
mptcp_do_fallback(ssk);
984985
return false;
985986
}
986987

net/mptcp/protocol.c

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -623,10 +623,9 @@ static bool mptcp_check_data_fin(struct sock *sk)
623623

624624
static void mptcp_dss_corruption(struct mptcp_sock *msk, struct sock *ssk)
625625
{
626-
if (READ_ONCE(msk->allow_infinite_fallback)) {
626+
if (mptcp_try_fallback(ssk)) {
627627
MPTCP_INC_STATS(sock_net(ssk),
628628
MPTCP_MIB_DSSCORRUPTIONFALLBACK);
629-
mptcp_do_fallback(ssk);
630629
} else {
631630
MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DSSCORRUPTIONRESET);
632631
mptcp_subflow_reset(ssk);
@@ -889,6 +888,14 @@ static bool __mptcp_finish_join(struct mptcp_sock *msk, struct sock *ssk)
889888
if (sk->sk_state != TCP_ESTABLISHED)
890889
return false;
891890

891+
spin_lock_bh(&msk->fallback_lock);
892+
if (__mptcp_check_fallback(msk)) {
893+
spin_unlock_bh(&msk->fallback_lock);
894+
return false;
895+
}
896+
mptcp_subflow_joined(msk, ssk);
897+
spin_unlock_bh(&msk->fallback_lock);
898+
892899
/* attach to msk socket only after we are sure we will deal with it
893900
* at close time
894901
*/
@@ -897,7 +904,6 @@ static bool __mptcp_finish_join(struct mptcp_sock *msk, struct sock *ssk)
897904

898905
mptcp_subflow_ctx(ssk)->subflow_id = msk->subflow_id++;
899906
mptcp_sockopt_sync_locked(msk, ssk);
900-
mptcp_subflow_joined(msk, ssk);
901907
mptcp_stop_tout_timer(sk);
902908
__mptcp_propagate_sndbuf(sk, ssk);
903909
return true;
@@ -1236,10 +1242,14 @@ static void mptcp_update_infinite_map(struct mptcp_sock *msk,
12361242
mpext->infinite_map = 1;
12371243
mpext->data_len = 0;
12381244

1245+
if (!mptcp_try_fallback(ssk)) {
1246+
mptcp_subflow_reset(ssk);
1247+
return;
1248+
}
1249+
12391250
MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_INFINITEMAPTX);
12401251
mptcp_subflow_ctx(ssk)->send_infinite_map = 0;
12411252
pr_fallback(msk);
1242-
mptcp_do_fallback(ssk);
12431253
}
12441254

12451255
#define MPTCP_MAX_GSO_SIZE (GSO_LEGACY_MAX_SIZE - (MAX_TCP_HEADER + 1))
@@ -2643,9 +2653,9 @@ static void mptcp_check_fastclose(struct mptcp_sock *msk)
26432653

26442654
static void __mptcp_retrans(struct sock *sk)
26452655
{
2656+
struct mptcp_sendmsg_info info = { .data_lock_held = true, };
26462657
struct mptcp_sock *msk = mptcp_sk(sk);
26472658
struct mptcp_subflow_context *subflow;
2648-
struct mptcp_sendmsg_info info = {};
26492659
struct mptcp_data_frag *dfrag;
26502660
struct sock *ssk;
26512661
int ret, err;
@@ -2690,6 +2700,18 @@ static void __mptcp_retrans(struct sock *sk)
26902700
info.sent = 0;
26912701
info.limit = READ_ONCE(msk->csum_enabled) ? dfrag->data_len :
26922702
dfrag->already_sent;
2703+
2704+
/*
2705+
* make the whole retrans decision, xmit, disallow
2706+
* fallback atomic
2707+
*/
2708+
spin_lock_bh(&msk->fallback_lock);
2709+
if (__mptcp_check_fallback(msk)) {
2710+
spin_unlock_bh(&msk->fallback_lock);
2711+
release_sock(ssk);
2712+
return;
2713+
}
2714+
26932715
while (info.sent < info.limit) {
26942716
ret = mptcp_sendmsg_frag(sk, ssk, dfrag, &info);
26952717
if (ret <= 0)
@@ -2705,6 +2727,7 @@ static void __mptcp_retrans(struct sock *sk)
27052727
info.size_goal);
27062728
WRITE_ONCE(msk->allow_infinite_fallback, false);
27072729
}
2730+
spin_unlock_bh(&msk->fallback_lock);
27082731

27092732
release_sock(ssk);
27102733
}
@@ -2841,6 +2864,7 @@ static void __mptcp_init_sock(struct sock *sk)
28412864
msk->last_ack_recv = tcp_jiffies32;
28422865

28432866
mptcp_pm_data_init(msk);
2867+
spin_lock_init(&msk->fallback_lock);
28442868

28452869
/* re-use the csk retrans timer for MPTCP-level retrans */
28462870
timer_setup(&msk->sk.icsk_retransmit_timer, mptcp_retransmit_timer, 0);
@@ -3637,7 +3661,13 @@ bool mptcp_finish_join(struct sock *ssk)
36373661

36383662
/* active subflow, already present inside the conn_list */
36393663
if (!list_empty(&subflow->node)) {
3664+
spin_lock_bh(&msk->fallback_lock);
3665+
if (__mptcp_check_fallback(msk)) {
3666+
spin_unlock_bh(&msk->fallback_lock);
3667+
return false;
3668+
}
36403669
mptcp_subflow_joined(msk, ssk);
3670+
spin_unlock_bh(&msk->fallback_lock);
36413671
mptcp_propagate_sndbuf(parent, ssk);
36423672
return true;
36433673
}

net/mptcp/protocol.h

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,10 @@ struct mptcp_sock {
346346
u32 subflow_id;
347347
u32 setsockopt_seq;
348348
char ca_name[TCP_CA_NAME_MAX];
349+
350+
spinlock_t fallback_lock; /* protects fallback and
351+
* allow_infinite_fallback
352+
*/
349353
};
350354

351355
#define mptcp_data_lock(sk) spin_lock_bh(&(sk)->sk_lock.slock)
@@ -1188,15 +1192,21 @@ static inline bool mptcp_check_fallback(const struct sock *sk)
11881192
return __mptcp_check_fallback(msk);
11891193
}
11901194

1191-
static inline void __mptcp_do_fallback(struct mptcp_sock *msk)
1195+
static inline bool __mptcp_try_fallback(struct mptcp_sock *msk)
11921196
{
11931197
if (__mptcp_check_fallback(msk)) {
11941198
pr_debug("TCP fallback already done (msk=%p)\n", msk);
1195-
return;
1199+
return true;
11961200
}
1197-
if (WARN_ON_ONCE(!READ_ONCE(msk->allow_infinite_fallback)))
1198-
return;
1201+
spin_lock_bh(&msk->fallback_lock);
1202+
if (!msk->allow_infinite_fallback) {
1203+
spin_unlock_bh(&msk->fallback_lock);
1204+
return false;
1205+
}
1206+
11991207
set_bit(MPTCP_FALLBACK_DONE, &msk->flags);
1208+
spin_unlock_bh(&msk->fallback_lock);
1209+
return true;
12001210
}
12011211

12021212
static inline bool __mptcp_has_initial_subflow(const struct mptcp_sock *msk)
@@ -1208,14 +1218,15 @@ static inline bool __mptcp_has_initial_subflow(const struct mptcp_sock *msk)
12081218
TCPF_SYN_RECV | TCPF_LISTEN));
12091219
}
12101220

1211-
static inline void mptcp_do_fallback(struct sock *ssk)
1221+
static inline bool mptcp_try_fallback(struct sock *ssk)
12121222
{
12131223
struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
12141224
struct sock *sk = subflow->conn;
12151225
struct mptcp_sock *msk;
12161226

12171227
msk = mptcp_sk(sk);
1218-
__mptcp_do_fallback(msk);
1228+
if (!__mptcp_try_fallback(msk))
1229+
return false;
12191230
if (READ_ONCE(msk->snd_data_fin_enable) && !(ssk->sk_shutdown & SEND_SHUTDOWN)) {
12201231
gfp_t saved_allocation = ssk->sk_allocation;
12211232

@@ -1227,6 +1238,7 @@ static inline void mptcp_do_fallback(struct sock *ssk)
12271238
tcp_shutdown(ssk, SEND_SHUTDOWN);
12281239
ssk->sk_allocation = saved_allocation;
12291240
}
1241+
return true;
12301242
}
12311243

12321244
#define pr_fallback(a) pr_debug("%s:fallback to TCP (msk=%p)\n", __func__, a)
@@ -1236,7 +1248,7 @@ static inline void mptcp_subflow_early_fallback(struct mptcp_sock *msk,
12361248
{
12371249
pr_fallback(msk);
12381250
subflow->request_mptcp = 0;
1239-
__mptcp_do_fallback(msk);
1251+
WARN_ON_ONCE(!__mptcp_try_fallback(msk));
12401252
}
12411253

12421254
static inline bool mptcp_check_infinite_map(struct sk_buff *skb)

net/mptcp/subflow.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -543,9 +543,11 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
543543
mptcp_get_options(skb, &mp_opt);
544544
if (subflow->request_mptcp) {
545545
if (!(mp_opt.suboptions & OPTION_MPTCP_MPC_SYNACK)) {
546+
if (!mptcp_try_fallback(sk))
547+
goto do_reset;
548+
546549
MPTCP_INC_STATS(sock_net(sk),
547550
MPTCP_MIB_MPCAPABLEACTIVEFALLBACK);
548-
mptcp_do_fallback(sk);
549551
pr_fallback(msk);
550552
goto fallback;
551553
}
@@ -1383,7 +1385,7 @@ static bool subflow_check_data_avail(struct sock *ssk)
13831385
return true;
13841386
}
13851387

1386-
if (!READ_ONCE(msk->allow_infinite_fallback)) {
1388+
if (!mptcp_try_fallback(ssk)) {
13871389
/* fatal protocol error, close the socket.
13881390
* subflow_error_report() will introduce the appropriate barriers
13891391
*/
@@ -1399,8 +1401,6 @@ static bool subflow_check_data_avail(struct sock *ssk)
13991401
WRITE_ONCE(subflow->data_avail, false);
14001402
return false;
14011403
}
1402-
1403-
mptcp_do_fallback(ssk);
14041404
}
14051405

14061406
skb = skb_peek(&ssk->sk_receive_queue);
@@ -1665,7 +1665,6 @@ int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_local *local,
16651665
/* discard the subflow socket */
16661666
mptcp_sock_graft(ssk, sk->sk_socket);
16671667
iput(SOCK_INODE(sf));
1668-
WRITE_ONCE(msk->allow_infinite_fallback, false);
16691668
mptcp_stop_tout_timer(sk);
16701669
return 0;
16711670

@@ -1845,7 +1844,7 @@ static void subflow_state_change(struct sock *sk)
18451844

18461845
msk = mptcp_sk(parent);
18471846
if (subflow_simultaneous_connect(sk)) {
1848-
mptcp_do_fallback(sk);
1847+
WARN_ON_ONCE(!mptcp_try_fallback(sk));
18491848
pr_fallback(msk);
18501849
subflow->conn_finished = 1;
18511850
mptcp_propagate_state(parent, sk, subflow, NULL);

0 commit comments

Comments
 (0)