Skip to content

Commit

Permalink
mptcp: fix integer overflow in mptcp_subflow_discard_data()
Browse files Browse the repository at this point in the history
jira LE-1907
Rebuild_History Non-Buildable kernel-4.18.0-294.el8
commit-author Paolo Abeni <pabeni@redhat.com>
commit 1d39cd8
Empty-Commit: Cherry-Pick Conflicts during history rebuild.
Will be included in final tarball splat. Ref for failed cherry-pick at:
ciq/ciq_backports/kernel-4.18.0-294.el8/1d39cd8c.failed

Christoph reported an infinite loop in the subflow receive path
under stress condition.

If there are multiple subflows, each of them using a large send
buffer, the delta between the sequence number used by
MPTCP-level retransmission can and the current msk->ack_seq
can be greater than MAX_INT.

In the above scenario, when calling mptcp_subflow_discard_data(),
such delta will be truncated to int, and could result in a negative
number: no bytes will be dropped, and subflow_check_data_avail()
will try again to process the same packet, looping forever.

This change addresses the issue by expanding the 'limit' size to 64
bits, so that overflows are not possible anymore.

Closes: multipath-tcp/mptcp_net-next#87
Fixes: 6719331 ("mptcp: trigger msk processing even for OoO data")
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
	Signed-off-by: Paolo Abeni <pabeni@redhat.com>
	Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
	Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 1d39cd8)
	Signed-off-by: Jonathan Maple <jmaple@ciq.com>

# Conflicts:
#	net/mptcp/subflow.c
  • Loading branch information
PlaidCat committed Sep 11, 2024
1 parent 28a28b3 commit 8cb9873
Showing 1 changed file with 80 additions and 0 deletions.
80 changes: 80 additions & 0 deletions ciq/ciq_backports/kernel-4.18.0-294.el8/1d39cd8c.failed
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
mptcp: fix integer overflow in mptcp_subflow_discard_data()

jira LE-1907
Rebuild_History Non-Buildable kernel-4.18.0-294.el8
commit-author Paolo Abeni <pabeni@redhat.com>
commit 1d39cd8cf75f79d082ee97f5fd2a6286bcc692c1
Empty-Commit: Cherry-Pick Conflicts during history rebuild.
Will be included in final tarball splat. Ref for failed cherry-pick at:
ciq/ciq_backports/kernel-4.18.0-294.el8/1d39cd8c.failed

Christoph reported an infinite loop in the subflow receive path
under stress condition.

If there are multiple subflows, each of them using a large send
buffer, the delta between the sequence number used by
MPTCP-level retransmission can and the current msk->ack_seq
can be greater than MAX_INT.

In the above scenario, when calling mptcp_subflow_discard_data(),
such delta will be truncated to int, and could result in a negative
number: no bytes will be dropped, and subflow_check_data_avail()
will try again to process the same packet, looping forever.

This change addresses the issue by expanding the 'limit' size to 64
bits, so that overflows are not possible anymore.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/87
Fixes: 6719331c2f73 ("mptcp: trigger msk processing even for OoO data")
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 1d39cd8cf75f79d082ee97f5fd2a6286bcc692c1)
Signed-off-by: Jonathan Maple <jmaple@ciq.com>

# Conflicts:
# net/mptcp/subflow.c
diff --cc net/mptcp/subflow.c
index 825e0c8e0ea5,34d6230df017..000000000000
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@@ -769,16 -806,25 +769,21 @@@ validate_seq
return MAPPING_OK;
}

++<<<<<<< HEAD
+static int subflow_read_actor(read_descriptor_t *desc,
+ struct sk_buff *skb,
+ unsigned int offset, size_t len)
++=======
+ static void mptcp_subflow_discard_data(struct sock *ssk, struct sk_buff *skb,
+ u64 limit)
++>>>>>>> 1d39cd8cf75f (mptcp: fix integer overflow in mptcp_subflow_discard_data())
{
- struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
- bool fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN;
- u32 incr;
-
- incr = limit >= skb->len ? skb->len + fin : limit;
-
- pr_debug("discarding=%d len=%d seq=%d", incr, skb->len,
- subflow->map_subflow_seq);
- MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_DUPDATA);
- tcp_sk(ssk)->copied_seq += incr;
- 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)
- subflow->map_valid = 0;
- if (incr)
- tcp_cleanup_rbuf(ssk, incr);
+ size_t copy_len = min(desc->count, len);
+
+ desc->count -= copy_len;
+
+ pr_debug("flushed %zu bytes, %zu left", copy_len, desc->count);
+ return copy_len;
}

static bool subflow_check_data_avail(struct sock *ssk)
* Unmerged path net/mptcp/subflow.c

0 comments on commit 8cb9873

Please sign in to comment.