Re: [PATCH] mptcp: generate subflow hmac after mptcp_finish_join()

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

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