From owner-freebsd-net@FreeBSD.ORG Sat Nov 6 10:49:19 2004 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8E69F16A4CE for ; Sat, 6 Nov 2004 10:49:19 +0000 (GMT) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id B1E6A43D1F for ; Sat, 6 Nov 2004 10:49:18 +0000 (GMT) (envelope-from andre@freebsd.org) Received: (qmail 13698 invoked from network); 6 Nov 2004 10:45:04 -0000 Received: from unknown (HELO freebsd.org) ([62.48.0.54]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 6 Nov 2004 10:45:04 -0000 Message-ID: <418CAC2D.41A7B5DD@freebsd.org> Date: Sat, 06 Nov 2004 11:49:17 +0100 From: Andre Oppermann X-Mailer: Mozilla 4.8 [en] (Windows NT 5.0; U) X-Accept-Language: en MIME-Version: 1.0 To: James References: <20041106102539.GA30766@scylla.towardex.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit cc: freebsd-net@freebsd.org Subject: Re: ip_fastforward() sanity check.. X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Nov 2004 10:49:19 -0000 James wrote: > > I seem to have a little concern in one specific early-sanity check in the > ip_fastforward() function of the latest 5.3 code base: > > /* > * Is first mbuf large enough for ip header and is header present? > */ > if (m->m_len < sizeof (struct ip) && > (m = m_pullup(m, sizeof (struct ip))) == 0) { > ipstat.ips_toosmall++; > goto drop; > } > > Okay, if m_pullup() returns 0 due to failure, it already called m_freem(m) by > itself. But we have "goto drop;" after that, which is redundant, no? > > I don't think this is a bit of issue in IPv4 implementation, but as obviously, > in IPv6 implementation, if calling 'goto drop' or redundant m_freem(m) in case > where m_pullup returns NULL/0, it may crash the kernel rock hard at > m_tag_delete_chain in uipc_mbuf.c (even if you are checking 'if (m) m_freem(m)' > as remains are left over) > > If any one has any comments, please let me know. If this is not a concern > please disregard my rant and excuse me for waste of time :) This is indeed a bug. Fixed in ip_fastfwd.c rev 1.24 a couple of minutes ago. Thanks for reporting. -- Andre