[PATCH mptcp-next v3 2/3] mptcp: pm: skip extra_subflows accounting on disconnected msk

[Tao Cui <cui.tao@linux.dev>]

From: Tao Cui <cuitao@kylinos.cn>

The WARN_ON_ONCE() guards added to the extra_subflows decrement sites
turn out to be reachable:

mptcp_pm_data_reset() zeroes the counter with only the msk socket lock
held, while an MP_JOIN subflow can still sit in msk->join_list, its
reference already accounted by mptcp_pm_allow_new_subflow() under
pm->lock. If the socket gets disconnected(AF_UNSPEC) in that window,
mptcp_pm_data_reset() zeroes the counter, and the join list is flushed
later at release_sock() time: the leftover subflow then reaches
mptcp_pm_subflow_check_next() (or __mptcp_pm_close_subflow() for
kernel PM sockets) with the counter already at 0, firing the warning.
On panic_on_warn kernels this is a remotely triggerable panic, which
is worse than the silent wrap the guards replaced.

Skip the PM accounting when the msk is already in TCP_CLOSE: in the
scenario above the state is set before the counters are cleared, and
once the msk is closed the accounting is not relevant anymore. Keep a
clamp and a rate-limited pr_warn() on the decrement sites instead,
to leave a trace of any imbalance we would still not know about.

Fixes: e99c1ca89071 ("mptcp: pm: add WARN_ON_ONCE guards on extra_subflows underflow")
Link: https://github.com/multipath-tcp/mptcp_net-next/issues/629
Suggested-by: Matthieu Baerts <matttbe@kernel.org>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 net/mptcp/pm.c       | 11 ++++++++++-
 net/mptcp/protocol.h | 14 ++++++++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 07cdcdb54b15..cccf5319cce0 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -671,9 +671,18 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
 	update_subflows = subflow->request_join || subflow->mp_join;
 	if (mptcp_pm_is_userspace(msk)) {
 		if (update_subflows) {
+			/* The PM counters have already been cleared if the
+			 * msk got disconnected while this subflow was still
+			 * queued in the join list
+			 */
+			if (inet_sk_state_load(sk) == TCP_CLOSE)
+				return;
 			spin_lock_bh(&pm->lock);
-			if (!WARN_ON_ONCE(pm->extra_subflows == 0))
+			if (likely(pm->extra_subflows))
 				pm->extra_subflows--;
+			else
+				pr_warn_ratelimited("extra_subflows underflow, msk=%p\n",
+						    msk);
 			spin_unlock_bh(&pm->lock);
 		}
 		return;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 87ccb84e9927..07821551499c 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1252,8 +1252,18 @@ u8 mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk);
 /* called under PM lock */
 static inline void __mptcp_pm_close_subflow(struct mptcp_sock *msk)
 {
-	if (!WARN_ON_ONCE(msk->pm.extra_subflows == 0) &&
-	    --msk->pm.extra_subflows < mptcp_pm_get_limit_extra_subflows(msk))
+	/* The PM counters have already been cleared if the msk got
+	 * disconnected while this subflow was still queued in the
+	 * join list
+	 */
+	if (inet_sk_state_load((struct sock *)msk) == TCP_CLOSE)
+		return;
+
+	if (unlikely(msk->pm.extra_subflows == 0)) {
+		pr_warn_ratelimited("extra_subflows underflow, msk=%p\n", msk);
+		return;
+	}
+	if (--msk->pm.extra_subflows < mptcp_pm_get_limit_extra_subflows(msk))
 		WRITE_ONCE(msk->pm.accept_subflow, true);
 }
 
-- 
2.43.0