* [PATCH] mptcp: generate subflow hmac after mptcp_finish_join()
@ 2021-05-07 8:02 Jianguo Wu
2021-05-13 0:38 ` Mat Martineau
2021-05-13 7:50 ` Matthieu Baerts
0 siblings, 2 replies; 3+ messages in thread
From: Jianguo Wu @ 2021-05-07 8:02 UTC (permalink / raw)
To: mptcp; +Cc: Florian Westphal, Paolo Abeni, Matthieu Baerts
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(-)
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mptcp: generate subflow hmac after mptcp_finish_join()
2021-05-07 8:02 [PATCH] mptcp: generate subflow hmac after mptcp_finish_join() Jianguo Wu
@ 2021-05-13 0:38 ` Mat Martineau
2021-05-13 7:50 ` Matthieu Baerts
1 sibling, 0 replies; 3+ messages in thread
From: Mat Martineau @ 2021-05-13 0:38 UTC (permalink / raw)
To: Jianguo Wu; +Cc: mptcp, Florian Westphal, Paolo Abeni, Matthieu Baerts
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mptcp: generate subflow hmac after mptcp_finish_join()
2021-05-07 8:02 [PATCH] mptcp: generate subflow hmac after mptcp_finish_join() Jianguo Wu
2021-05-13 0:38 ` Mat Martineau
@ 2021-05-13 7:50 ` Matthieu Baerts
1 sibling, 0 replies; 3+ messages in thread
From: Matthieu Baerts @ 2021-05-13 7:50 UTC (permalink / raw)
To: Jianguo Wu, Mat Martineau; +Cc: Florian Westphal, Paolo Abeni, mptcp
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
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-05-13 7:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07 8:02 [PATCH] mptcp: generate subflow hmac after mptcp_finish_join() Jianguo Wu
2021-05-13 0:38 ` Mat Martineau
2021-05-13 7:50 ` Matthieu Baerts
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.