mbox series

[net,0/5] tcp: more robust ooo handling

Message ID 20180723162821.11556-1-edumazet@google.com
Headers show
Series tcp: more robust ooo handling | expand

Message

Eric Dumazet July 23, 2018, 4:28 p.m. UTC
Juha-Matti Tilli reported that malicious peers could inject tiny
packets in out_of_order_queue, forcing very expensive calls
to tcp_collapse_ofo_queue() and tcp_prune_ofo_queue() for
every incoming packet.

With tcp_rmem[2] default of 6MB, the ooo queue could
contain ~7000 nodes.

This patch series makes sure we cut cpu cycles enough to
render the attack not critical.

We might in the future go further, like disconnecting
or black-holing proven malicious flows.

Eric Dumazet (5):
  tcp: free batches of packets in tcp_prune_ofo_queue()
  tcp: avoid collapses in tcp_prune_queue() if possible
  tcp: detect malicious patterns in tcp_collapse_ofo_queue()
  tcp: call tcp_drop() from tcp_data_queue_ofo()
  tcp: add tcp_ooo_try_coalesce() helper

 net/ipv4/tcp_input.c | 62 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 50 insertions(+), 12 deletions(-)

Comments

David Miller July 23, 2018, 7:03 p.m. UTC | #1
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 23 Jul 2018 09:28:16 -0700

> Juha-Matti Tilli reported that malicious peers could inject tiny
> packets in out_of_order_queue, forcing very expensive calls
> to tcp_collapse_ofo_queue() and tcp_prune_ofo_queue() for
> every incoming packet.
> 
> With tcp_rmem[2] default of 6MB, the ooo queue could
> contain ~7000 nodes.
> 
> This patch series makes sure we cut cpu cycles enough to
> render the attack not critical.
> 
> We might in the future go further, like disconnecting
> or black-holing proven malicious flows.

Sucky...

It took me a while to understand the sums_tiny logic, every
time I read that function I forget that we reset all of the
state and restart the loop after a coalesce inside the loop.

Series applied, and queued up for -stable.

Thanks!
David Woodhouse Aug. 3, 2018, 10:55 a.m. UTC | #2
On Mon, 2018-07-23 at 12:03 -0700, David Miller wrote:
> From: Eric Dumazet <edumazet@google.com>
> Date: Mon, 23 Jul 2018 09:28:16 -0700
> 
> > Juha-Matti Tilli reported that malicious peers could inject tiny
> > packets in out_of_order_queue, forcing very expensive calls
> > to tcp_collapse_ofo_queue() and tcp_prune_ofo_queue() for
> > every incoming packet.
> > 
> > With tcp_rmem[2] default of 6MB, the ooo queue could
> > contain ~7000 nodes.
> > 
> > This patch series makes sure we cut cpu cycles enough to
> > render the attack not critical.
> > 
> > We might in the future go further, like disconnecting
> > or black-holing proven malicious flows.
> 
> Sucky...
> 
> It took me a while to understand the sums_tiny logic, every
> time I read that function I forget that we reset all of the
> state and restart the loop after a coalesce inside the loop.
> 
> Series applied, and queued up for -stable.

I see the first four in 4.9.116 but not the fifth (adding
tcp_ooo_try_coalesce()).

Is that intentional?
David Miller Aug. 3, 2018, 11:53 p.m. UTC | #3
From: David Woodhouse <dwmw2@infradead.org>
Date: Fri, 03 Aug 2018 11:55:37 +0100

> I see the first four in 4.9.116 but not the fifth (adding
> tcp_ooo_try_coalesce()).
> 
> Is that intentional? 

I don't work on the 4.9 -stable backports, so I personally have
no idea.

I submitted for 4.17 and 4.14
Greg KH Aug. 4, 2018, 7:05 a.m. UTC | #4
On Fri, Aug 03, 2018 at 04:53:27PM -0700, David Miller wrote:
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Fri, 03 Aug 2018 11:55:37 +0100
> 
> > I see the first four in 4.9.116 but not the fifth (adding
> > tcp_ooo_try_coalesce()).
> > 
> > Is that intentional? 
> 
> I don't work on the 4.9 -stable backports, so I personally have
> no idea.
> 
> I submitted for 4.17 and 4.14

Ok, then it's my fault :)

Odds are it did not apply and so I didn't backport it.  If you think it
should be there, please provide a working backport.

thanks,

greg k-h
David Woodhouse Aug. 4, 2018, 9:04 a.m. UTC | #5
On Sat, 2018-08-04 at 09:05 +0200, Greg KH wrote:
> 
> Ok, then it's my fault :)
> 
> Odds are it did not apply and so I didn't backport it.  If you think it
> should be there, please provide a working backport.

It has whitespace issues but that's about it. Will send a version which
applies cleanly...