Skip to content

Commit

Permalink
mptcp: be careful on subflows shutdown
Browse files Browse the repository at this point in the history
When the workqueue disposes of the msk, the subflows can still
receive some data from the peer after __mptcp_close_ssk()
completes.

The above could trigger a race between the msk receive path and the
msk destruction. Acquiring the mptcp_data_lock() in __mptcp_destroy_sock()
will not save the day: the rx path could be reached even after msk
destruction completes.

Instead use the subflow 'disposable' flag to prevent entering
the msk receive path after __mptcp_close_ssk().

Fixes: e16163b ("mptcp: refactor shutdown and close")
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Paolo Abeni authored and David S. Miller committed Dec 10, 2020
1 parent 0597d0f commit d7b1bfd
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions net/mptcp/protocol.c
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,13 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
int sk_rbuf, ssk_rbuf;
bool wake;

/* The peer can send data while we are shutting down this
* subflow at msk destruction time, but we must avoid enqueuing
* more data to the msk receive queue
*/
if (unlikely(subflow->disposable))
return;

/* move_skbs_to_msk below can legitly clear the data_avail flag,
* but we will need later to properly woke the reader, cache its
* value
Expand Down Expand Up @@ -2119,15 +2126,16 @@ void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
sock_orphan(ssk);
}

subflow->disposable = 1;

/* if ssk hit tcp_done(), tcp_cleanup_ulp() cleared the related ops
* the ssk has been already destroyed, we just need to release the
* reference owned by msk;
*/
if (!inet_csk(ssk)->icsk_ulp_ops) {
kfree_rcu(subflow, rcu);
} else {
/* otherwise ask tcp do dispose of ssk and subflow ctx */
subflow->disposable = 1;
/* otherwise tcp will dispose of the ssk and subflow ctx */
__tcp_close(ssk, 0);

/* close acquired an extra ref */
Expand Down

0 comments on commit d7b1bfd

Please sign in to comment.