Skip to content

Commit 0b13a81

Browse files
Paolo Abenimatttbe
Paolo Abeni
authored andcommitted
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 <matthieu.baerts@tessares.net> Tested-by: Matthieu Baerts <matthieu.baerts@tessares.net> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
1 parent 67e842b commit 0b13a81

File tree

1 file changed

+28
-8
lines changed

1 file changed

+28
-8
lines changed

net/mptcp/protocol.c

+28-8
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,10 @@ static void __mptcp_update_wmem(struct sock *sk)
947947
{
948948
struct mptcp_sock *msk = mptcp_sk(sk);
949949

950+
#ifdef CONFIG_LOCKDEP
951+
WARN_ON_ONCE(!lockdep_is_held(&sk->sk_lock.slock));
952+
#endif
953+
950954
if (!msk->wmem_reserved)
951955
return;
952956

@@ -1027,7 +1031,7 @@ static void dfrag_clear(struct sock *sk, struct mptcp_data_frag *dfrag)
10271031
put_page(dfrag->page);
10281032
}
10291033

1030-
static void __mptcp_clean_una(struct sock *sk)
1034+
static bool __mptcp_do_clean_una(struct sock *sk)
10311035
{
10321036
struct mptcp_sock *msk = mptcp_sk(sk);
10331037
struct mptcp_data_frag *dtmp, *dfrag;
@@ -1068,19 +1072,22 @@ static void __mptcp_clean_una(struct sock *sk)
10681072
}
10691073

10701074
out:
1071-
if (cleaned) {
1072-
if (tcp_under_memory_pressure(sk)) {
1073-
__mptcp_update_wmem(sk);
1074-
sk_mem_reclaim_partial(sk);
1075-
}
1076-
}
10771075

10781076
if (snd_una == READ_ONCE(msk->snd_nxt)) {
10791077
if (msk->timer_ival && !mptcp_data_fin_enabled(msk))
10801078
mptcp_stop_timer(sk);
10811079
} else {
10821080
mptcp_reset_timer(sk);
10831081
}
1082+
return cleaned;
1083+
}
1084+
1085+
static void __mptcp_clean_una(struct sock *sk)
1086+
{
1087+
if (__mptcp_do_clean_una(sk) && tcp_under_memory_pressure(sk)) {
1088+
__mptcp_update_wmem(sk);
1089+
sk_mem_reclaim_partial(sk);
1090+
}
10841091
}
10851092

10861093
static void __mptcp_clean_una_wakeup(struct sock *sk)
@@ -1089,6 +1096,19 @@ static void __mptcp_clean_una_wakeup(struct sock *sk)
10891096
mptcp_write_space(sk);
10901097
}
10911098

1099+
/* variant __mptcp_clean_una_wakeup() for caller owning the msk socket lock,
1100+
* but not the msk_data_lock/msk socket spin lock
1101+
*/
1102+
static void mptcp_clean_una_wakeup(struct sock *sk)
1103+
{
1104+
#ifdef CONFIG_LOCKDEP
1105+
WARN_ON_ONCE(!lockdep_is_held(&sk->sk_lock));
1106+
#endif
1107+
if (__mptcp_do_clean_una(sk) && tcp_under_memory_pressure(sk))
1108+
mptcp_mem_reclaim_partial(sk);
1109+
mptcp_write_space(sk);
1110+
}
1111+
10921112
static void mptcp_enter_memory_pressure(struct sock *sk)
10931113
{
10941114
struct mptcp_subflow_context *subflow;
@@ -2299,7 +2319,7 @@ static void __mptcp_retrans(struct sock *sk)
22992319
struct sock *ssk;
23002320
int ret;
23012321

2302-
__mptcp_clean_una_wakeup(sk);
2322+
mptcp_clean_una_wakeup(sk);
23032323
dfrag = mptcp_rtx_head(sk);
23042324
if (!dfrag) {
23052325
if (mptcp_data_fin_enabled(msk)) {

0 commit comments

Comments
 (0)