All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [MPTCP] Re: [PATCH 2/2] mptcp: drop MP_JOIN request sock on syn cookies
@ 2020-06-12  5:14 Mat Martineau
  0 siblings, 0 replies; only message in thread
From: Mat Martineau @ 2020-06-12  5:14 UTC (permalink / raw
  To: mptcp 

[-- Attachment #1: Type: text/plain, Size: 2840 bytes --]

On Thu, 11 Jun 2020, Paolo Abeni wrote:

> Currently any MPTCP socket using syn cookies will fallback to
> TCP at 3rd ack time. In case of MP_JOIN requests, the RFC mandate
> closing the child and sockets, but the existing error paths
> do not handle the syncookie scenario correctly.
>
> Address the issue always forcing the child shutdown in case of
> MP_JOIN fallback.
>
> Fixes: ae2dd7164943 ("mptcp: handle tcp fallback when using syn cookies")
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> ---
> net/mptcp/subflow.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 8367d93e2f5c..339c9226c6ee 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -393,22 +393,25 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
> 	struct mptcp_subflow_context *listener = mptcp_subflow_ctx(sk);
> 	struct mptcp_subflow_request_sock *subflow_req;
> 	struct mptcp_options_received mp_opt;
> -	bool fallback_is_fatal = false;
> +	bool fallback, fallback_is_fatal;
> 	struct sock *new_msk = NULL;
> -	bool fallback = false;
> 	struct sock *child;
>
> 	pr_debug("listener=%p, req=%p, conn=%p", listener, req, listener->conn);
>
> -	/* we need later a valid 'mp_capable' value even when options are not
> -	 * parsed
> +	/* After child creation we must look for 'mp_capable' even when options
> +	 * are not parsed
> 	 */
> 	mp_opt.mp_capable = 0;
> -	if (tcp_rsk(req)->is_mptcp == 0)
> +
> +	/* hopefully temporary handling for MP_JOIN+syncookie */
> +	subflow_req = mptcp_subflow_rsk(req);
> +	fallback_is_fatal = subflow_req->mp_join;
> +	fallback = !tcp_rsk(req)->is_mptcp;
> +	if (fallback)
> 		goto create_child;
>
> 	/* if the sk is MP_CAPABLE, we try to fetch the client key */
> -	subflow_req = mptcp_subflow_rsk(req);
> 	if (subflow_req->mp_capable) {
> 		if (TCP_SKB_CB(skb)->seq != subflow_req->ssn_offset + 1) {
> 			/* here we can receive and accept an in-window,
> @@ -429,12 +432,11 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
> 		if (!new_msk)
> 			fallback = true;
> 	} else if (subflow_req->mp_join) {
> -		fallback_is_fatal = true;
> 		mptcp_get_options(skb, &mp_opt);
> 		if (!mp_opt.mp_join ||
> 		    !subflow_hmac_valid(req, &mp_opt)) {
> 			SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_JOINACKMAC);
> -			return NULL;
> +			fallback = true;
> 		}
> 	}

When fallback and fallback_is_fatal are both true when continuing past 
this block of code, it does look difficult to detect when to *not* send a 
reset without doing the work of allocating the child. The approach of 
doing the allocation seems ok, hopefully the extra work to do the 
allocation is not itself subject to attack :)


--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2020-06-12  5:14 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-12  5:14 [MPTCP] Re: [PATCH 2/2] mptcp: drop MP_JOIN request sock on syn cookies Mat Martineau

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.