Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 02 Jul 2011 14:24:37 -0700
From:      Colin Percival <cperciva@freebsd.org>
To:        freebsd-net@freebsd.org, Jack F Vogel <jfv@FreeBSD.org>
Subject:   integer overflow in TCP LRO
Message-ID:  <4E0F8C95.50507@freebsd.org>

next in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------070300000001080603080107
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Hi all,

In tcp_lro_rx it's possible for lro->len to exceed 65536, resulting in an
integer overflow and 65536 bytes of TCP "packet loss" when tcp_lro_flush
stuffs lro->len back into an IP header.

It's clear that an attempt was made to avoid overflow
339:                        /* flush packet if required */
340:                        device_mtu = cntl->ifp->if_mtu;
341:                        if (lro->len > (65535 - device_mtu)) {
but this doesn't work because incoming "packets" can be larger than
device_mtu bytes if LRO is turned on.

I've attached a patch which fixes this and improves Linux->FreeBSD network
performance on EC2 cluster compute nodes from 13 Mbps to 4100 Mbps... any
objections to me committing this?

-- 
Colin Percival
Security Officer, FreeBSD | freebsd.org | The power to serve
Founder / author, Tarsnap | tarsnap.com | Online backups for the truly paranoid

--------------070300000001080603080107
Content-Type: text/x-patch;
 name="tcp_lro.c.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="tcp_lro.c.diff"

--- tcp_lro.c.orig	2011-07-02 19:53:51.000000000 +0000
+++ tcp_lro.c	2011-07-02 18:12:31.000000000 +0000
@@ -274,6 +274,14 @@
 		    lro->dest_port == tcp->th_dport &&
 		    lro->source_ip == ip->ip_src.s_addr && 
 		    lro->dest_ip == ip->ip_dst.s_addr) {
+			/* Flush now if appending will result in overflow. */
+			if (lro->len > (65535 - tcp_data_len)) {
+				SLIST_REMOVE(&cntl->lro_active, lro,
+					     lro_entry, next);
+				tcp_lro_flush(cntl, lro);
+				break;
+			}
+
 			/* Try to append it */
 
 			if (__predict_false(seq != lro->next_seq)) {

--------------070300000001080603080107--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E0F8C95.50507>