| Message ID | 20210222161815.27469-1-fw@strlen.de |
|---|---|
| State | Accepted, archived |
| Commit | 15d29d726a6eefb520f3a7a50174a4142cc3404a |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [v2] mptcp: reset 'first' and ack_hint on subflow close | expand |
Hi Florian, Paolo, On 22/02/2021 17:18, Florian Westphal wrote: > Just like with last_snd, we have to NULL 'first' on subflow close. > > ack_hint isn't strictly required (its never dereferenced), but better to > clear this explicitly as well instead of making it an exception. > > msk->first is dereferenced unconditionally at accept time, but > at that point the ssk is not on the conn_list yet -- this means > worker can't see it when iterating the conn_list. > > Reported-by: Paolo Abeni <pabeni@redhat.com> > Signed-off-by: Florian Westphal <fw@strlen.de> Thank you for this patch! Just applied in our tree, after "mptcp: dispose initial struct socket when its subflow is closed" with my RvB tag. - 15d29d726a6e: mptcp: reset 'first' and ack_hint on subflow close - Results: b5622a37311b..45795e8f868e Tests + export are in progress! Cheers, Matt
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 6b4eac26256f..e3865d59009b 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2161,6 +2161,12 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk, if (ssk == msk->last_snd) msk->last_snd = NULL; + if (ssk == msk->ack_hint) + msk->ack_hint = NULL; + + if (ssk == msk->first) + msk->first = NULL; + if (msk->subflow && ssk == msk->subflow->sk) mptcp_dispose_initial_subflow(msk); } @@ -3298,6 +3304,9 @@ static int mptcp_stream_accept(struct socket *sock, struct socket *newsock, /* PM/worker can now acquire the first subflow socket * lock without racing with listener queue cleanup, * we can notify it, if needed. + * + * Even if remote has reset the initial subflow by now + * the refcnt is still at least one. */ subflow = mptcp_subflow_ctx(msk->first); list_add(&subflow->node, &msk->conn_list);
Just like with last_snd, we have to NULL 'first' on subflow close. ack_hint isn't strictly required (its never dereferenced), but better to clear this explicitly as well instead of making it an exception. msk->first is dereferenced unconditionally at accept time, but at that point the ssk is not on the conn_list yet -- this means worker can't see it when iterating the conn_list. Reported-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de> --- v2: add comment to clarify msk->first access in accept() is safe even if tcp connection has been closed already. net/mptcp/protocol.c | 9 +++++++++ 1 file changed, 9 insertions(+)