Skip to content

Commit

Permalink
mptcp: fix possible memory leak on syn_recv race
Browse files Browse the repository at this point in the history
When subflow_syn_recv_sock() loses the inet hash insert race, the
newly created children will be released by inet_csk_complete_hashdance().

In that scenario, without any further hint, the ulp release callback
will keep the ulp context alive, expecting that the msk socket will
later free it.

Anyway the dying child is not linked to any msk socket, and the context
will be leaked, as reported by Christoph.

Address the issue explicitly releasing the context in the critical
scenario.

Fixes: cec37a6 ("mptcp: Handle MP_CAPABLE options for outgoing connections")
Reported-by: Christoph Paasch <[email protected]>
Closes: #356
Signed-off-by: Paolo Abeni <[email protected]>
Reviewed-by: Matthieu Baerts <[email protected]>
  • Loading branch information
Paolo Abeni authored and matttbe committed Mar 7, 2023
1 parent 9f5eda2 commit ad01610
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion net/mptcp/subflow.c
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
child = listener->icsk_af_ops->syn_recv_sock(sk, skb, req, dst,
req_unhash, own_req);

if (child && *own_req) {
if (likely(child && *own_req)) {
struct mptcp_subflow_context *ctx = mptcp_subflow_ctx(child);

tcp_rsk(req)->drop_req = false;
Expand Down Expand Up @@ -898,6 +898,12 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKRX);
tcp_rsk(req)->drop_req = true;
}
} else if (child) {
/* inet_csk_complete_hashdance() is going to drop the sock
* soon, but context must be explicitly deleted or will be
* leaked
*/
mptcp_subflow_drop_ctx(child);
}

out:
Expand Down

0 comments on commit ad01610

Please sign in to comment.