[MPTCP] Re: [PATCH net] mptcp: fix data_fin handing in RX path

[MPTCP] Re: [PATCH net] mptcp: fix data_fin handing in RX path

[Mat Martineau]

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


On Tue, 21 Apr 2020, Paolo Abeni wrote:

> The data fin flag is set only via a DSS option, but currently
> mptcp_incoming_options() copies it inconditionally from the
> provided RX options.
>
> Since the tcp sock RX options are not explicitly cleared on
> socket free/alloc cycle, we can end-up with a stray data_fin
> value while parsing e.g. MPC packets.
>
> That would lead to mapping data corruption and will trigger
> a few WARN_ON() in the RX path.
>
> Instead of adding a costly memset(), fetch the data_fin flag
> only for DSS packets - when we always expicitly initialize
> such bit at option parsing time.
>
> Fixes: 648ef4b88673 ("mptcp: Implement MPTCP receive path")
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> ---
> net/mptcp/options.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 3d541b60dcf3..129e9c214b2d 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -878,12 +878,11 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb,
> 			mpext->data_seq = mp_opt->data_seq;
> 			mpext->subflow_seq = mp_opt->subflow_seq;
> 			mpext->dsn64 = mp_opt->dsn64;
> +			mpext->data_fin = mp_opt->data_fin;
> 		}
> 		mpext->data_len = mp_opt->data_len;
> 		mpext->use_map = 1;
> 	}
> -
> -	mpext->data_fin = mp_opt->data_fin;
> }
>
> void mptcp_write_options(__be32 *ptr, struct mptcp_out_options *opts)

Thanks for fixing that, Paolo. Looks good to me.

Reviewed-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>


--
Mat Martineau
Intel

[MPTCP] Re: [PATCH net] mptcp: fix data_fin handing in RX path

[David Miller]

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

From: Paolo Abeni <pabeni(a)redhat.com>
Date: Wed, 22 Apr 2020 18:24:56 +0200

> The data fin flag is set only via a DSS option, but
> mptcp_incoming_options() copies it unconditionally from the
> provided RX options.
> 
> Since we do not clear all the mptcp sock RX options in a
> socket free/alloc cycle, we can end-up with a stray data_fin
> value while parsing e.g. MPC packets.
> 
> That would lead to mapping data corruption and will trigger
> a few WARN_ON() in the RX path.
> 
> Instead of adding a costly memset(), fetch the data_fin flag
> only for DSS packets - when we always explicitly initialize
> such bit at option parsing time.
> 
> Fixes: 648ef4b88673 ("mptcp: Implement MPTCP receive path")
> Reviewed-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>

Applied and queued up for v5.6 -stable, thanks.