From owner-freebsd-hackers@FreeBSD.ORG Sat Jan 15 01:45:53 2011 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3C060106566B for ; Sat, 15 Jan 2011 01:45:53 +0000 (UTC) (envelope-from sec@42.org) Received: from ice.42.org (v6.42.org [IPv6:2001:608:9::1]) by mx1.freebsd.org (Postfix) with ESMTP id EA9C88FC0A for ; Sat, 15 Jan 2011 01:45:52 +0000 (UTC) Received: by ice.42.org (Postfix, from userid 1000) id 26D4C28459; Sat, 15 Jan 2011 02:45:52 +0100 (CET) Date: Sat, 15 Jan 2011 02:45:52 +0100 From: Stefan `Sec` Zehl To: freebsd-hackers@freebsd.org Message-ID: <20110115014551.GA24844@ice.42.org> X-Current-Backlog: 3891 messages Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4.2.3i I-love-doing-this: really X-Modeline: vim:set ts=8 sw=4 smarttab tw=72 si noic notitle: Accept-Languages: de, en X-URL: http://sec.42.org/ Subject: tcp_output.c:560 - is this code correct? X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 15 Jan 2011 01:45:53 -0000 Hi, I just found a bug in FreeBSD amd64 -- (see the PR#kern/154006) a missing "(long)" cast in tcp_output.c. I have a question about this code (around line 560) where adv is calculated: | if (recwin > 0 && !(tp->t_flags & TF_NEEDSYN) && | !TCPS_HAVERCVDFIN(tp->t_state)) { | /* | * "adv" is the amount we can increase the window, | * taking into account that we are limited by | * TCP_MAXWIN << tp->rcv_scale. | */ | long adv = min(recwin, (long)TCP_MAXWIN << tp->rcv_scale) - | (tp->rcv_adv - tp->rcv_nxt); | [...] adv is getting negative in my case (receiver window size == 0) because recwin == (tp->rcv_adv - tp->rcv_nxt), but recwin is bigger than (TCP_MAXWIN << tp->rcv_scale) >From the comment above it seems that adv shouldn't be negative ever. So I wonder if that code was meant to be: long adv = min(recwin - (tp->rcv_adv - tp->rcv_nxt) , (long)TCP_MAXWIN << tp->rcv_scale); instead? At least as far as I understand the code, that would make more sense. Thanks, Sec -- The problem with troubleshooting is that trouble shoots back.