From owner-freebsd-bugs@FreeBSD.ORG Sat Jan 15 06:10:10 2011 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D205A106566B for ; Sat, 15 Jan 2011 06:10:10 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id A6E318FC0A for ; Sat, 15 Jan 2011 06:10:10 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p0F6AAGp042439 for ; Sat, 15 Jan 2011 06:10:10 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p0F6AABn042436; Sat, 15 Jan 2011 06:10:10 GMT (envelope-from gnats) Date: Sat, 15 Jan 2011 06:10:10 GMT Message-Id: <201101150610.p0F6AABn042436@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Bruce Evans Cc: Subject: Re: kern/154006: tcp "window probe" bug on 64bit X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Bruce Evans List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 15 Jan 2011 06:10:10 -0000 The following reply was made to PR kern/154006; it has been noted by GNATS. From: Bruce Evans To: Bruce Evans Cc: Stefan `Sec` Zehl , freebsd-bugs@freebsd.org, FreeBSD-gnats-submit@freebsd.org Subject: Re: kern/154006: tcp "window probe" bug on 64bit Date: Sat, 15 Jan 2011 17:05:02 +1100 (EST) On Sat, 15 Jan 2011, Bruce Evans wrote: > ... > Later code in tcp_output uses bogus casts to long and larger code instead: > > % if (recwin < (long)(tp->rcv_adv - tp->rcv_nxt)) > % recwin = (long)(tp->rcv_adv - tp->rcv_nxt); > % if (recwin > (long)TCP_MAXWIN << tp->rcv_scale) > % recwin = (long)TCP_MAXWIN << tp->rcv_scale; > % ... > % if (recwin > 0 && SEQ_GT(tp->rcv_nxt + recwin, tp->rcv_adv)) > % tp->rcv_adv = tp->rcv_nxt + recwin; > > Note that the first statement avoids using the technically incorrect > SEQ_FOO() although its internals are better (cast to int instead of > long). It uses cases essentially like yours. Then further analysis ^^^^^ a cast > is simpler because everything is converted to long. The second starement > is similar to the first half of the broken expression. Large code using > if's and else's and tests (x >= y) before subtracting y from x is much > easier to get right than 1 complicated 1-statement expression like the > broken one. It takes these (x >= y) tests to make code with mixed types > obviously correct. But I prefer small fast code with ints for everything, > since type analyis is too hard. But the casts to long are not good. Here they have no effect except to break the warning about the bad type of `recwin'. recwin has type long, so assignment to it does the same conversion as the cast, possibly with a warning about the implicit conversion if it might overflow (can overflow only on 32-bit arches). The code depends on rcv_adv being sequentially >= rcv_next with or without the cast. Otherwise, the difference is huge unsigned int, and the cast only changes this (by benign overflow) on 32-bit arches. This can be fixed by casting to int instead of long (now the cast may have an effect, and breaking the warning may be intential), or my proposed SEQ_DIFF() macros work well here: int recwin, sd; ... sd = SEQ_NONNEG_NONLARGE_DIFF(tp->rcv_adv, tp->rcv_nxt); /* * SEQ_DIFF() supports negative differences; * SEQ_NONNEG_NONGLARGE_DIFF() KASSERT()s that they don't happen and * are not too large. This name is too long. */ if (recwin < sd) recwin = sd; ... Bruce