Commit 56a666c4 authored by Paolo Abeni's avatar Paolo Abeni Committed by Jakub Kicinski

mptcp: fix possible list corruption on passive MPJ

At passive MPJ time, if the msk socket lock is held by the user,
the new subflow is appended to the msk->join_list under the msk
data lock.

In mptcp_release_cb()/__mptcp_flush_join_list(), the subflows in
that list are moved from the join_list into the conn_list under the
msk socket lock.

Append and removal could race, possibly corrupting such list.
Address the issue splicing the join list into a temporary one while
still under the msk data lock.

Found by code inspection, the race itself should be almost impossible
to trigger in practice.

Fixes: 3e501490 ("mptcp: cleanup MPJ subflow list handling")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
Reviewed-by: default avatarMatthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: default avatarMatthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 0ad529d9
...@@ -850,12 +850,12 @@ static bool __mptcp_finish_join(struct mptcp_sock *msk, struct sock *ssk) ...@@ -850,12 +850,12 @@ static bool __mptcp_finish_join(struct mptcp_sock *msk, struct sock *ssk)
return true; return true;
} }
static void __mptcp_flush_join_list(struct sock *sk) static void __mptcp_flush_join_list(struct sock *sk, struct list_head *join_list)
{ {
struct mptcp_subflow_context *tmp, *subflow; struct mptcp_subflow_context *tmp, *subflow;
struct mptcp_sock *msk = mptcp_sk(sk); struct mptcp_sock *msk = mptcp_sk(sk);
list_for_each_entry_safe(subflow, tmp, &msk->join_list, node) { list_for_each_entry_safe(subflow, tmp, join_list, node) {
struct sock *ssk = mptcp_subflow_tcp_sock(subflow); struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
bool slow = lock_sock_fast(ssk); bool slow = lock_sock_fast(ssk);
...@@ -3342,9 +3342,14 @@ static void mptcp_release_cb(struct sock *sk) ...@@ -3342,9 +3342,14 @@ static void mptcp_release_cb(struct sock *sk)
for (;;) { for (;;) {
unsigned long flags = (msk->cb_flags & MPTCP_FLAGS_PROCESS_CTX_NEED) | unsigned long flags = (msk->cb_flags & MPTCP_FLAGS_PROCESS_CTX_NEED) |
msk->push_pending; msk->push_pending;
struct list_head join_list;
if (!flags) if (!flags)
break; break;
INIT_LIST_HEAD(&join_list);
list_splice_init(&msk->join_list, &join_list);
/* the following actions acquire the subflow socket lock /* the following actions acquire the subflow socket lock
* *
* 1) can't be invoked in atomic scope * 1) can't be invoked in atomic scope
...@@ -3355,8 +3360,9 @@ static void mptcp_release_cb(struct sock *sk) ...@@ -3355,8 +3360,9 @@ static void mptcp_release_cb(struct sock *sk)
msk->push_pending = 0; msk->push_pending = 0;
msk->cb_flags &= ~flags; msk->cb_flags &= ~flags;
spin_unlock_bh(&sk->sk_lock.slock); spin_unlock_bh(&sk->sk_lock.slock);
if (flags & BIT(MPTCP_FLUSH_JOIN_LIST)) if (flags & BIT(MPTCP_FLUSH_JOIN_LIST))
__mptcp_flush_join_list(sk); __mptcp_flush_join_list(sk, &join_list);
if (flags & BIT(MPTCP_PUSH_PENDING)) if (flags & BIT(MPTCP_PUSH_PENDING))
__mptcp_push_pending(sk, 0); __mptcp_push_pending(sk, 0);
if (flags & BIT(MPTCP_RETRANSMIT)) if (flags & BIT(MPTCP_RETRANSMIT))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment