Subject: Re: [GIT]: Networking

From: William Allen Simpson <[email-masked]>
Date: Sat, 12 Dec 2009 05:10:32 -0500

David Miller wrote:
> David S. Miller (4):
>       Merge branch 'master' of git://git.kernel.org/.../linville/wireless-next-2.6
>       tcp: Remove runtime check that can never be true.

This is a poor patch that was never sent to the netdev list for review.  Copies to
and from user space should always check for buffer overruns.  A better patch was
submitted (attached) some time ago.

Miller's patch has 2 flaws:

  1) It accidentally removed a comment line that describes the purpose of the
following memcpy(), paralleling language in the internet-draft.

  2) It fails to test for buffer overruns.  The existing code has an accurate test,
that compiles (to no code) correctly with current gcc.  But it produces a warning
in some (fairly old) gcc versions.

Instead, the better patch uses the Kernel defined BUILD_BUG_ON().  Please
substitute the better patch in the main tree.

>       Merge branch 'master' of git://git.kernel.org/.../linville/wireless-2.6
>       Merge branch 'master' of /home/davem/src/GIT/linux-2.6/
> 
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c8666b7..3d2229c 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2541,10 +2541,7 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
 			ctd.tcpct_s_data_desired = cvp->s_data_desired;
 
 			/* Cookie(s) saved, return as nonce */
-			if (sizeof(ctd.tcpct_value) < cvp->cookie_pair_size) {
-				/* impossible? */
-				return -EINVAL;
-			}
+			BUILD_BUG_ON(sizeof(ctd.tcpct_value) < TCP_COOKIE_PAIR_SIZE);
 			memcpy(&ctd.tcpct_value[0], &cvp->cookie_pair[0],
 			       cvp->cookie_pair_size);
 			ctd.tcpct_used = cvp->cookie_pair_size;
-- 
1.6.3.3