[BUG] before() integer overflow

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: <netdev@...>
Date: Tuesday, August 5, 2008 - 1:19 pm

Hello!


In include/net/tcp.h, the before() function is defined like this :

 241 /*
 242  * The next routines deal with comparing 32 bit unsigned ints
 243  * and worry about wraparound (automatic with unsigned arithmetic).
 244  */
 245 
 246 static inline int before(__u32 seq1, __u32 seq2)
 247 {
 248         return (__s32)(seq1-seq2) < 0;
 249 }
 250 #define after(seq2, seq1)   before(seq1, seq2)


If seq1 = 0xffffff and seq2 = 0 (so seq1 > seq2), the difference is
equal to 0xffffff, or -1 as a 32 bits signed number.

 => before() will return true instead of false.

It's not really a big deal[1], but I didn't understand why my invalid
packets were accepted when playing with Netfilter code.

If I'm not wrong, a trivial patch could be :

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 8983386..2b01227 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -248,7 +248,7 @@ extern int tcp_memory_pressure;
 
 static inline int before(__u32 seq1, __u32 seq2)
 {
-        return (__s32)(seq1-seq2) < 0;
+        return ((__u64)seq1-seq2) < 0;
 }
 #define after(seq2, seq1)      before(seq1, seq2)

Thanks


Footnotes: 
[1]  The TCP sequence number space is divided by two, now on 31 bits,
     phear! :) 
-- 
Nicolas Bareil                                  http://chdir.org/~nico/
OpenPGP=0xAE4F7057 Fingerprint=34DB22091049FB2F33E6B71580F314DAAE4F7057

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[BUG] before() integer overflow, Nicolas Bareil, (Tue Aug 5, 1:19 pm)
Re: [BUG] before() integer overflow, David Stevens, (Tue Aug 5, 1:51 pm)
Re: [BUG] before() integer overflow, Nicolas Bareil, (Tue Aug 5, 2:24 pm)
Re: [BUG] before() integer overflow, Ben Hutchings, (Tue Aug 5, 1:40 pm)