mptcp: plug races between subflow fail and subflow creation

commit def5b7b2643ebba696fc60ddf675dca13f073486 upstream.

We have races similar to the one addressed by the previous patch between
subflow failing and additional subflow creation. They are just harder to
trigger.

The solution is similar. Use a separate flag to track the condition
'socket state prevent any additional subflow creation' protected by the
fallback lock.

The socket fallback makes such flag true, and also receiving or sending
an MP_FAIL option.

The field 'allow_infinite_fallback' is now always touched under the
relevant lock, we can drop the ONCE annotation on write.

Fixes: 478d770008b0 ("mptcp: send out MP_FAIL when data checksum fails")
Cc: stable@vger.kernel.org
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-2-391aff963322@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 31747f974941fae3f80bfc8313e82c41f92562eb..b6870dc04095dc9b0fec162aade147a812e5ab26 100644 (file)
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -761,8 +761,14 @@ void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq)
 
        pr_debug("fail_seq=%llu\n", fail_seq);
 
-       if (!READ_ONCE(msk->allow_infinite_fallback))
+       /* After accepting the fail, we can't create any other subflows */
+       spin_lock_bh(&msk->fallback_lock);
+       if (!msk->allow_infinite_fallback) {
+               spin_unlock_bh(&msk->fallback_lock);
                return;
+       }
+       msk->allow_subflows = false;
+       spin_unlock_bh(&msk->fallback_lock);
 
        if (!subflow->fail_tout) {
                pr_debug("send MP_FAIL response and infinite map\n");

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index ab5cd4e34365bf1f02ecf1289af0a467d0f7c02e..6725e99a3d71305464bf234a90872b70d6cb21fe 100644 (file)
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -789,7 +789,7 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
 static void mptcp_subflow_joined(struct mptcp_sock *msk, struct sock *ssk)
 {
        mptcp_subflow_ctx(ssk)->map_seq = READ_ONCE(msk->ack_seq);
-       WRITE_ONCE(msk->allow_infinite_fallback, false);
+       msk->allow_infinite_fallback = false;
        mptcp_event(MPTCP_EVENT_SUB_ESTABLISHED, msk, ssk, GFP_ATOMIC);
 }
 
@@ -801,7 +801,7 @@ static bool __mptcp_finish_join(struct mptcp_sock *msk, struct sock *ssk)
                return false;
 
        spin_lock_bh(&msk->fallback_lock);
-       if (__mptcp_check_fallback(msk)) {
+       if (!msk->allow_subflows) {
                spin_unlock_bh(&msk->fallback_lock);
                return false;
        }
@@ -2623,7 +2623,7 @@ static void __mptcp_retrans(struct sock *sk)
                                len = max(copied, len);
                                tcp_push(ssk, 0, info.mss_now, tcp_sk(ssk)->nonagle,
                                         info.size_goal);
-                               WRITE_ONCE(msk->allow_infinite_fallback, false);
+                               msk->allow_infinite_fallback = false;
                        }
                        spin_unlock_bh(&msk->fallback_lock);
 
@@ -2751,7 +2751,8 @@ static void __mptcp_init_sock(struct sock *sk)
        WRITE_ONCE(msk->first, NULL);
        inet_csk(sk)->icsk_sync_mss = mptcp_sync_mss;
        WRITE_ONCE(msk->csum_enabled, mptcp_is_checksum_enabled(sock_net(sk)));
-       WRITE_ONCE(msk->allow_infinite_fallback, true);
+       msk->allow_infinite_fallback = true;
+       msk->allow_subflows = true;
        msk->recovery = false;
        msk->subflow_id = 1;
        msk->last_data_sent = tcp_jiffies32;
@@ -3547,7 +3548,7 @@ bool mptcp_finish_join(struct sock *ssk)
        /* active subflow, already present inside the conn_list */
        if (!list_empty(&subflow->node)) {
                spin_lock_bh(&msk->fallback_lock);
-               if (__mptcp_check_fallback(msk)) {
+               if (!msk->allow_subflows) {
                        spin_unlock_bh(&msk->fallback_lock);
                        return false;
                }

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 4f2a41b1c8b861651ad41fd407ee00d4a89194fd..833efc079b20f34d0d32b1591f06c0b308833731 100644 (file)
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -346,13 +346,15 @@ struct mptcp_sock {
                u64     rtt_us; /* last maximum rtt of subflows */
        } rcvq_space;
        u8              scaling_ratio;
+       bool            allow_subflows;
 
        u32             subflow_id;
        u32             setsockopt_seq;
        char            ca_name[TCP_CA_NAME_MAX];
 
-       spinlock_t      fallback_lock;  /* protects fallback and
-                                        * allow_infinite_fallback
+       spinlock_t      fallback_lock;  /* protects fallback,
+                                        * allow_infinite_fallback and
+                                        * allow_join
                                         */
 };
 
@@ -1224,6 +1226,7 @@ static inline bool __mptcp_try_fallback(struct mptcp_sock *msk)
                return false;
        }
 
+       msk->allow_subflows = false;
        set_bit(MPTCP_FALLBACK_DONE, &msk->flags);
        spin_unlock_bh(&msk->fallback_lock);
        return true;

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index f463f1b4c6c42e2db8a66319da4308f0c7939ac5..8a4159b0642d962e5101899e00c705d0f160d455 100644 (file)
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1304,20 +1304,29 @@ static void subflow_sched_work_if_closed(struct mptcp_sock *msk, struct sock *ss
                mptcp_schedule_work(sk);
 }
 
-static void mptcp_subflow_fail(struct mptcp_sock *msk, struct sock *ssk)
+static bool mptcp_subflow_fail(struct mptcp_sock *msk, struct sock *ssk)
 {
        struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
        unsigned long fail_tout;
 
+       /* we are really failing, prevent any later subflow join */
+       spin_lock_bh(&msk->fallback_lock);
+       if (!msk->allow_infinite_fallback) {
+               spin_unlock_bh(&msk->fallback_lock);
+               return false;
+       }
+       msk->allow_subflows = false;
+       spin_unlock_bh(&msk->fallback_lock);
+
        /* graceful failure can happen only on the MPC subflow */
        if (WARN_ON_ONCE(ssk != READ_ONCE(msk->first)))
-               return;
+               return false;
 
        /* since the close timeout take precedence on the fail one,
         * no need to start the latter when the first is already set
         */
        if (sock_flag((struct sock *)msk, SOCK_DEAD))
-               return;
+               return true;
 
        /* we don't need extreme accuracy here, use a zero fail_tout as special
         * value meaning no fail timeout at all;
@@ -1329,6 +1338,7 @@ static void mptcp_subflow_fail(struct mptcp_sock *msk, struct sock *ssk)
        tcp_send_ack(ssk);
 
        mptcp_reset_tout_timer(msk, subflow->fail_tout);
+       return true;
 }
 
 static bool subflow_check_data_avail(struct sock *ssk)
@@ -1389,12 +1399,11 @@ fallback:
                    (subflow->mp_join || subflow->valid_csum_seen)) {
                        subflow->send_mp_fail = 1;
 
-                       if (!READ_ONCE(msk->allow_infinite_fallback)) {
+                       if (!mptcp_subflow_fail(msk, ssk)) {
                                subflow->reset_transient = 0;
                                subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
                                goto reset;
                        }
-                       mptcp_subflow_fail(msk, ssk);
                        WRITE_ONCE(subflow->data_avail, true);
                        return true;
                }