Bug#421283: [PATCH] Fix wrong checksum for split TCP packets on 64-bit MIPS
Package: linux-2.6
This is needed for our 2.6.18 kernel in etch.
----- Forwarded message from Dave Johnson <djohnson+linux-mips@sw.starentnetworks.com> -----
From: Dave Johnson <djohnson+linux-mips@sw.starentnetworks.com>
Subject: [PATCH] Fix wrong checksum for split TCP packets on 64-bit MIPS
Date: Wed, 18 Apr 2007 10:39:41 -0400
To: linux-mips@linux-mips.org, Ralf Baechle <ralf@linux-mips.org>
X-Mailer: VM 7.17 under 21.4 (patch 17) "Jumbo Shrimp" XEmacs Lucid
I've traced down an off-by-one TCP checksum calculation error under
the following conditions:
1) The TCP code needs to split a full-sized packet due to a reduced
MSS (typically due to the addition of TCP options mid-stream like
SACK).
_AND_
2) The checksum of the 2nd fragment is larger than the checksum of the
original packet. After subtraction this results in a checksum for
the 1st fragment with bits 16..31 set to 1. (this is ok)
_AND_
3) The checksum of the 1st fragment's TCP header plus the previously
32bit checksum of the 1st fragment DOES NOT cause a 32bit overflow
when added together. This results in a checksum of the TCP header
plus TCP data that still has the upper 16 bits as 1's.
_THEN_
4) The TCP+data checksum is added to the checksum of the pseudo IP
header with csum_tcpudp_nofold() incorrectly (the bug).
The problem is the checksum of the TCP+data is passed to
csum_tcpudp_nofold() as an 32bit unsigned value, however the assembly
code acts on it as if it is a 64bit unsigned value.
This causes an incorrect 32->64bit extension if the sum has bit 31
set. The resulting checksum is off by one.
This problems is data and TCP header dependent due to #2 and #3
above so it doesn't occur on every TCP packet split.
Patch below is against 2.6.20, however this problem looks like it's
been around since at least Aug 2003 when the 64bit ASM was added to
csum_tcpudp_nofold().
Signed-off-by: Dave Johnson <djohnson+linux-mips@sw.starentnetworks.com>
--- old/include/asm-mips/checksum.h 2007-01-24 14:23:22 -05:00
+++ new/include/asm-mips/checksum.h 2007-04-18 10:31:27 -04:00
@@ -166,7 +166,7 @@
#else
"r" (proto + len),
#endif
- "r" (sum));
+ "r" ((__force unsigned long)sum));
return sum;
}
----- End forwarded message -----
----- Forwarded message from Ralf Baechle <ralf@linux-mips.org> -----
From: Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH] Fix wrong checksum for split TCP packets on 64-bit MIPS
Date: Wed, 18 Apr 2007 17:17:07 +0100
To: Dave Johnson <djohnson+linux-mips@sw.starentnetworks.com>
Cc: linux-mips@linux-mips.org
User-Agent: Mutt/1.4.2.2i
On Wed, Apr 18, 2007 at 10:39:41AM -0400, Dave Johnson wrote:
> the following conditions:
>
> 1) The TCP code needs to split a full-sized packet due to a reduced
> MSS (typically due to the addition of TCP options mid-stream like
> SACK).
> _AND_
> 2) The checksum of the 2nd fragment is larger than the checksum of the
> original packet. After subtraction this results in a checksum for
> the 1st fragment with bits 16..31 set to 1. (this is ok)
> _AND_
> 3) The checksum of the 1st fragment's TCP header plus the previously
> 32bit checksum of the 1st fragment DOES NOT cause a 32bit overflow
> when added together. This results in a checksum of the TCP header
> plus TCP data that still has the upper 16 bits as 1's.
> _THEN_
> 4) The TCP+data checksum is added to the checksum of the pseudo IP
> header with csum_tcpudp_nofold() incorrectly (the bug).
>
> The problem is the checksum of the TCP+data is passed to
> csum_tcpudp_nofold() as an 32bit unsigned value, however the assembly
> code acts on it as if it is a 64bit unsigned value.
>
> This causes an incorrect 32->64bit extension if the sum has bit 31
> set. The resulting checksum is off by one.
Sigh. The second bug of this kind. As clever and apparently elegent as
the sign extension stuff happens to look on MIPS as prone to unobvious
accidents it is at times. Oh well.
Applied & thanks!
Ralf
----- End forwarded message -----
--
Martin Michlmayr
http://www.cyrius.com/
Reply to: