From owner-freebsd-net@FreeBSD.ORG Tue Feb 1 14:53:11 2011 Return-Path: Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C2867106564A; Tue, 1 Feb 2011 14:53:11 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 92DDE8FC14; Tue, 1 Feb 2011 14:53:11 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 45FDF46B17; Tue, 1 Feb 2011 09:53:11 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.10]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 89E608A027; Tue, 1 Feb 2011 09:53:10 -0500 (EST) From: John Baldwin To: Lawrence Stewart Date: Tue, 1 Feb 2011 08:29:23 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.4-CBSD-20110107; KDE/4.4.5; amd64; ; ) References: <201101311217.07073.jhb@freebsd.org> <4D477289.8040901@freebsd.org> In-Reply-To: <4D477289.8040901@freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201102010829.24043.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Tue, 01 Feb 2011 09:53:10 -0500 (EST) X-Virus-Scanned: clamav-milter 0.96.3 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=0.5 required=4.2 tests=BAYES_00,MAY_BE_FORGED, RDNS_DYNAMIC autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on bigwig.baldwin.cx Cc: "Bjoern A. Zeeb" , Andre Oppermann , net@freebsd.org Subject: Re: Bogus KASSERT() in tcp_output()? X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Feb 2011 14:53:11 -0000 On Monday, January 31, 2011 9:40:09 pm Lawrence Stewart wrote: > On 02/01/11 04:17, John Baldwin wrote: > > Somewhat related fallout to the bug reported on security@ recently, I think > > this KASSERT() in tcp_output() is bogus: > > > > > > KASSERT(len + hdrlen + ipoptlen == m_length(m, NULL), > > ("%s: mbuf chain shorter than expected", __func__)); > > > > Specifically, just a few lines earlier in tcp_output() we set the packet > > header length to just 'len + hdrlen': > > > > /* > > * Put TCP length in extended header, and then > > * checksum extended header and data. > > */ > > m->m_pkthdr.len = hdrlen + len; /* in6_cksum() need this */ > > > > Also, the ipoptions are stored in a separate mbuf chain in the in pcb > > (inp_options) that is passed as a separate argument to ip_output(). Given > > that, I would think that m_length() should not reflect ipoptlen since it > > should not include IP options in that chain? > > > > There is some relevant prior discussion on src-committers@ for r212803 > between Andre and Bjoern. I still don't see where ipoptlen bytes are reserved in the mbuf chain. After this block where 'm' is allocated and initialized: /* * Grab a header mbuf, attaching a copy of data to * be transmitted, and initialize the header from * the template for sends on this connection. */ if (len) { ... m->m_len = hdrlen; ... if (len <= MHLEN - hdrlen - max_linkhdr) { ... m->m_len += len; } else { m->m_next = m_copy(mb, moff, (int)len); ... } ... } else { ... m->m_len = hdrlen; } The length of the mbuf chain headed by 'm' is clearly hdrlen + len. At no point anywhere do we do any sort of m_prepend() or other operation to allocate space in the mbuf chain for the IP options. They are merged in in ip_output(). I think the only reason this KASSERT() isn't firing in HEAD is that IP options are rarely used? Is there an easy way to test a connection with IP options enabled with this KASSERT() enabled? -- John Baldwin