All of lore.kernel.org
 help / color / mirror / Atom feed
* [MPTCP] [PATCH net-next] mptcp: fix integer overflow in mptcp_subflow_discard_data()
@ 2020-09-17 21:07 ` Paolo Abeni
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Abeni @ 2020-09-17 21:07 UTC (permalink / raw)
  To: mptcp

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

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(a)apple.com>
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
net-next patch, as the culprit commit is only on net-next currently
---
 net/mptcp/subflow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index a2ae3087e24d..34d6230df017 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -807,7 +807,7 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
 }
 
 static void mptcp_subflow_discard_data(struct sock *ssk, struct sk_buff *skb,
-				       unsigned int limit)
+				       u64 limit)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
 	bool fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN;
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net-next] mptcp: fix integer overflow in mptcp_subflow_discard_data()
@ 2020-09-17 21:07 ` Paolo Abeni
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Abeni @ 2020-09-17 21:07 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, mptcp, Christoph Paasch

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>
---
net-next patch, as the culprit commit is only on net-next currently
---
 net/mptcp/subflow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index a2ae3087e24d..34d6230df017 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -807,7 +807,7 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
 }
 
 static void mptcp_subflow_discard_data(struct sock *ssk, struct sk_buff *skb,
-				       unsigned int limit)
+				       u64 limit)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
 	bool fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [MPTCP] Re: [PATCH net-next] mptcp: fix integer overflow in mptcp_subflow_discard_data()
  2020-09-17 21:07 ` Paolo Abeni
@ 2020-09-17 21:19 ` Mat Martineau
  -1 siblings, 0 replies; 6+ messages in thread
From: Mat Martineau @ 2020-09-17 21:19 UTC (permalink / raw)
  To: mptcp

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


On Thu, 17 Sep 2020, Paolo Abeni wrote:

> 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(a)apple.com>
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> ---
> net-next patch, as the culprit commit is only on net-next currently
> ---
> net/mptcp/subflow.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>

Thanks Paolo!

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


--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [MPTCP] [PATCH net-next] mptcp: fix integer overflow in mptcp_subflow_discard_data()
@ 2020-09-17 21:19 ` Mat Martineau
  0 siblings, 0 replies; 6+ messages in thread
From: Mat Martineau @ 2020-09-17 21:19 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, David S. Miller, mptcp


On Thu, 17 Sep 2020, Paolo Abeni wrote:

> 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>
> ---
> net-next patch, as the culprit commit is only on net-next currently
> ---
> net/mptcp/subflow.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>

Thanks Paolo!

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [MPTCP] Re: [PATCH net-next] mptcp: fix integer overflow in mptcp_subflow_discard_data()
  2020-09-17 21:07 ` Paolo Abeni
@ 2020-09-18  1:05 ` David Miller
  -1 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-09-18  1:05 UTC (permalink / raw)
  To: mptcp

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

From: Paolo Abeni <pabeni(a)redhat.com>
Date: Thu, 17 Sep 2020 23:07:24 +0200

> 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(a)apple.com>
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> ---
> net-next patch, as the culprit commit is only on net-next currently

Applied, thank you.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next] mptcp: fix integer overflow in mptcp_subflow_discard_data()
@ 2020-09-18  1:05 ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-09-18  1:05 UTC (permalink / raw)
  To: pabeni; +Cc: netdev, mptcp, cpaasch

From: Paolo Abeni <pabeni@redhat.com>
Date: Thu, 17 Sep 2020 23:07:24 +0200

> 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>
> ---
> net-next patch, as the culprit commit is only on net-next currently

Applied, thank you.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-18  1:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17 21:19 [MPTCP] Re: [PATCH net-next] mptcp: fix integer overflow in mptcp_subflow_discard_data() Mat Martineau
2020-09-17 21:19 ` [MPTCP] " Mat Martineau
  -- strict thread matches above, loose matches on Subject: below --
2020-09-18  1:05 [MPTCP] " David Miller
2020-09-18  1:05 ` David Miller
2020-09-17 21:07 [MPTCP] " Paolo Abeni
2020-09-17 21:07 ` Paolo Abeni

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.