Message ID | 79d42988-47da-dc8b-8416-4d4e76510f08@163.com |
---|---|
State | Accepted, archived |
Commit | cab4e45339c1d2ba98089947b9edd73b0d1a1b6e |
Delegated to: | Matthieu Baerts |
Headers | show |
Series | mptcp: generate subflow hmac after mptcp_finish_join() | expand |
On Fri, 7 May 2021, Jianguo Wu wrote: > From: Jianguo Wu <wujianguo@chinatelecom.cn> > > For outgoing subflow join, when recv SYNACK, in subflow_finish_connect(), > the mptcp_finish_join() may return false in some cases, and send a RESET > to remote, and no local hmac is required. > So generate subflow hmac after mptcp_finish_join(). > > Fixes: ec3edaa7ca6c ("mptcp: Add handling of outgoing MP_JOIN requests"). > Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn> > --- > net/mptcp/subflow.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Hi Jianguo - It looks like subflow->hmac is not accessed by any functions called from mptcp_finish_join(), and the selftests pass, so this looks ok for the export branch. Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com> Mat > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c > index c896803..554e7cce 100644 > --- a/net/mptcp/subflow.c > +++ b/net/mptcp/subflow.c > @@ -433,15 +433,15 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb) > goto do_reset; > } > > + if (!mptcp_finish_join(sk)) > + goto do_reset; > + > subflow_generate_hmac(subflow->local_key, subflow->remote_key, > subflow->local_nonce, > subflow->remote_nonce, > hmac); > memcpy(subflow->hmac, hmac, MPTCPOPT_HMAC_LEN); > > - if (!mptcp_finish_join(sk)) > - goto do_reset; > - > subflow->mp_join = 1; > MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_JOINSYNACKRX); > > -- > 1.8.3.1 > > > -- Mat Martineau Intel
Hi Jianguo, Mat, On 07/05/2021 10:02, Jianguo Wu wrote: > From: Jianguo Wu <wujianguo@chinatelecom.cn> > > For outgoing subflow join, when recv SYNACK, in subflow_finish_connect(), > the mptcp_finish_join() may return false in some cases, and send a RESET > to remote, and no local hmac is required. > So generate subflow hmac after mptcp_finish_join(). Should we cover this case with a packetdrill test? > Fixes: ec3edaa7ca6c ("mptcp: Add handling of outgoing MP_JOIN requests"). > Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn> Thank you for the patch and the review! Now in our tree, 'net' fix: - cab4e45339c1: mptcp: generate subflow hmac after mptcp_finish_join() - Results: 655ba588ab88..b8c6eee443a3 Builds and tests are now in progress: https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210513T075025 https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210513T075025 Cheers, Matt
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index c896803..554e7cce 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -433,15 +433,15 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb) goto do_reset; } + if (!mptcp_finish_join(sk)) + goto do_reset; + subflow_generate_hmac(subflow->local_key, subflow->remote_key, subflow->local_nonce, subflow->remote_nonce, hmac); memcpy(subflow->hmac, hmac, MPTCPOPT_HMAC_LEN); - if (!mptcp_finish_join(sk)) - goto do_reset; - subflow->mp_join = 1; MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_JOINSYNACKRX);