| Message ID | c79259641c0fda0a0df41c25f58ae0a1da1e3839.1613112149.git.geliangtang@gmail.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mat Martineau |
| Headers | show |
| Series | [net-next] mptcp: free resources when the port number is mismatched | expand |
On Fri, 12 Feb 2021, Geliang Tang wrote: > When the port number is mismatched with the announced ones, use > 'goto dispose_child' to free the resources instead of using 'goto out'. > > This patch also moves the port number checking code in > subflow_syn_recv_sock before the setting of ctx->conn, otherwise > subflow_drop_ctx will fail in dispose_child. > > Fixes: 5bc56388c74f ("mptcp: add port number check for MP_JOIN") > Reported-by: Paolo Abeni <pabeni@redhat.com> > Signed-off-by: Geliang Tang <geliangtang@gmail.com> > --- > net/mptcp/subflow.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c > index 280da418d60b..a9c8daf72768 100644 > --- a/net/mptcp/subflow.c > +++ b/net/mptcp/subflow.c > @@ -692,25 +692,25 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk, > if (!owner) > goto dispose_child; > > - /* move the msk reference ownership to the subflow */ > - subflow_req->msk = NULL; > - ctx->conn = (struct sock *)owner; What kind of failure did you see in subflow_drop_ctx() if ctx->conn is set before the port number check? If it's the sock_put(ctx->conn), that makes me wonder if there's an existing sock_hold/sock_put imbalance somewhere. Just trying to make sure we aren't hiding an existing bug here, since it looks like the above three lines shouldn't need to be moved, while these lines: > - if (!mptcp_finish_join(child)) > - goto dispose_child; > - > - SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKRX); > - tcp_rsk(req)->drop_req = true; > - ...do look like they belong after the port check. Mat > if (subflow_use_different_sport(owner, sk)) { > pr_debug("ack inet_sport=%d %d", > ntohs(inet_sk(sk)->inet_sport), > ntohs(inet_sk((struct sock *)owner)->inet_sport)); > if (!mptcp_pm_sport_in_anno_list(owner, sk)) { > SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MISMATCHPORTACKRX); > - goto out; > + goto dispose_child; > } > SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINPORTACKRX); > } > + > + /* move the msk reference ownership to the subflow */ > + subflow_req->msk = NULL; > + ctx->conn = (struct sock *)owner; > + if (!mptcp_finish_join(child)) > + goto dispose_child; > + > + SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKRX); > + tcp_rsk(req)->drop_req = true; > } > } > > -- > 2.29.2 > _______________________________________________ > mptcp mailing list -- mptcp@lists.01.org > To unsubscribe send an email to mptcp-leave@lists.01.org > -- Mat Martineau Intel
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 280da418d60b..a9c8daf72768 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -692,25 +692,25 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk, if (!owner) goto dispose_child; - /* move the msk reference ownership to the subflow */ - subflow_req->msk = NULL; - ctx->conn = (struct sock *)owner; - if (!mptcp_finish_join(child)) - goto dispose_child; - - SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKRX); - tcp_rsk(req)->drop_req = true; - if (subflow_use_different_sport(owner, sk)) { pr_debug("ack inet_sport=%d %d", ntohs(inet_sk(sk)->inet_sport), ntohs(inet_sk((struct sock *)owner)->inet_sport)); if (!mptcp_pm_sport_in_anno_list(owner, sk)) { SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MISMATCHPORTACKRX); - goto out; + goto dispose_child; } SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINPORTACKRX); } + + /* move the msk reference ownership to the subflow */ + subflow_req->msk = NULL; + ctx->conn = (struct sock *)owner; + if (!mptcp_finish_join(child)) + goto dispose_child; + + SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKRX); + tcp_rsk(req)->drop_req = true; } }
When the port number is mismatched with the announced ones, use 'goto dispose_child' to free the resources instead of using 'goto out'. This patch also moves the port number checking code in subflow_syn_recv_sock before the setting of ctx->conn, otherwise subflow_drop_ctx will fail in dispose_child. Fixes: 5bc56388c74f ("mptcp: add port number check for MP_JOIN") Reported-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: Geliang Tang <geliangtang@gmail.com> --- net/mptcp/subflow.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)