Re: [PATCH mptcp-net 2/2] mptcp: fix duplicate data handling

[Mat Martineau <martineau@kernel.org>]

On Thu, 25 Jul 2024, Paolo Abeni wrote:

> When a subflow receives and discards duplicate data, the mptcp
> stack assumes that the consumed offset inside the current skb is
> zero.
>
> With multiple subflows receiving data simultaneously such assertion
> does not held true. As a result the subflow-level copied_seq will
> be incorrectly increased and later on the same subflow will observe
> a bad mapping, leading to subflow reset.
>
> Address the issue tacking in account the skb consumed offset in

Just one fix but Matthieu can adjust: "Address the issue taking into 
account..."

Reviewed-by: Mat Martineau <martineau@kernel.org>

> mptcp_subflow_discard_data().
>
> Fixes: 04e4cd4f7ca4 ("mptcp: cleanup mptcp_subflow_discard_data()")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/subflow.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 0e4b5bfbeaa1..a21c712350c3 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1230,14 +1230,22 @@ static void mptcp_subflow_discard_data(struct sock *ssk, struct sk_buff *skb,
> {
> 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> 	bool fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN;
> -	u32 incr;
> +	struct tcp_sock *tp = tcp_sk(ssk);
> +	u32 offset, incr, avail_len;
>
> -	incr = limit >= skb->len ? skb->len + fin : limit;
> +	offset = tp->copied_seq - TCP_SKB_CB(skb)->seq;
> +	if (WARN_ON_ONCE(offset > skb->len))
> +		goto out;
> +
> +	avail_len = skb->len - offset;
> +	incr = limit >= avail_len ? avail_len + fin : limit;
>
> -	pr_debug("discarding=%d len=%d seq=%d", incr, skb->len,
> -		 subflow->map_subflow_seq);
> +	pr_debug("discarding=%d len=%d offset=%d seq=%d", incr, skb->len,
> +		 offset, subflow->map_subflow_seq);
> 	MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DUPDATA);
> 	tcp_sk(ssk)->copied_seq += incr;
> +
> +out:
> 	if (!before(tcp_sk(ssk)->copied_seq, TCP_SKB_CB(skb)->end_seq))
> 		sk_eat_skb(ssk, skb);
> 	if (mptcp_subflow_get_map_offset(subflow) >= subflow->map_data_len)
> --