mptcp: fix sk_forward_memory corruption under memory pressure

MPTCP sk_forward_memory handling is a bit special, as such field
is protected by the msk socket spin_lock, instead of the plain
socket lock.

Currently we have a code path updating such field without handling
the relevant lock:

__mptcp_retrans() -> __mptcp_clean_una_wakeup() -> __mptcp_update_wmem()

We can hit the above only under memory pressure. When that happen,
forward memory accounting could be corrupted, as reported by Matt.

Address the issue creating a new variant of __mptcp_clean_una_wakeup()
which handle fwd memory updates with the proper care and invoking
such new helper in the relevant code path.

Fixes: 64b9cea ("mptcp: fix spurious retransmissions")
Closes: #172
Reported-by: Matthieu Baerts <[email protected]>
Tested-by: Matthieu Baerts <[email protected]>
Reviewed-by: Mat Martineau <[email protected]>
Signed-off-by: Paolo Abeni <[email protected]>

net/mptcp/protocol.c

@@ -947,6 +947,10 @@ static void __mptcp_update_wmem(struct sock *sk)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
 
+#ifdef CONFIG_LOCKDEP
+	WARN_ON_ONCE(!lockdep_is_held(&sk->sk_lock.slock));
+#endif
+
 	if (!msk->wmem_reserved)
 		return;
 
@@ -1027,7 +1031,7 @@ static void dfrag_clear(struct sock *sk, struct mptcp_data_frag *dfrag)
 	put_page(dfrag->page);
 }
 
-static void __mptcp_clean_una(struct sock *sk)
+static bool __mptcp_do_clean_una(struct sock *sk)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct mptcp_data_frag *dtmp, *dfrag;
@@ -1068,19 +1072,22 @@ static void __mptcp_clean_una(struct sock *sk)
 	}
 
 out:
-	if (cleaned) {
-		if (tcp_under_memory_pressure(sk)) {
-			__mptcp_update_wmem(sk);
-			sk_mem_reclaim_partial(sk);
-		}
-	}
 
 	if (snd_una == READ_ONCE(msk->snd_nxt)) {
 		if (msk->timer_ival && !mptcp_data_fin_enabled(msk))
 			mptcp_stop_timer(sk);
 	} else {
 		mptcp_reset_timer(sk);
 	}
+	return cleaned;
+}
+
+static void __mptcp_clean_una(struct sock *sk)
+{
+	if (__mptcp_do_clean_una(sk) && tcp_under_memory_pressure(sk)) {
+		__mptcp_update_wmem(sk);
+		sk_mem_reclaim_partial(sk);
+	}
 }
 
 static void __mptcp_clean_una_wakeup(struct sock *sk)
@@ -1089,6 +1096,19 @@ static void __mptcp_clean_una_wakeup(struct sock *sk)
 	mptcp_write_space(sk);
 }
 
+/* variant __mptcp_clean_una_wakeup() for caller owning the msk socket lock,
+ * but not the msk_data_lock/msk socket spin lock
+ */
+static void mptcp_clean_una_wakeup(struct sock *sk)
+{
+#ifdef CONFIG_LOCKDEP
+	WARN_ON_ONCE(!lockdep_is_held(&sk->sk_lock));
+#endif
+	if (__mptcp_do_clean_una(sk) && tcp_under_memory_pressure(sk))
+		mptcp_mem_reclaim_partial(sk);
+	mptcp_write_space(sk);
+}
+
 static void mptcp_enter_memory_pressure(struct sock *sk)
 {
 	struct mptcp_subflow_context *subflow;
@@ -2299,7 +2319,7 @@ static void __mptcp_retrans(struct sock *sk)
 	struct sock *ssk;
 	int ret;
 
-	__mptcp_clean_una_wakeup(sk);
+	mptcp_clean_una_wakeup(sk);
 	dfrag = mptcp_rtx_head(sk);
 	if (!dfrag) {
 		if (mptcp_data_fin_enabled(msk)) {