From owner-svn-src-head@FreeBSD.ORG Mon Jun 22 22:54:43 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A63B11065680 for ; Mon, 22 Jun 2009 22:54:43 +0000 (UTC) (envelope-from andre@freebsd.org) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.freebsd.org (Postfix) with ESMTP id 78F2D8FC1D for ; Mon, 22 Jun 2009 22:54:40 +0000 (UTC) (envelope-from andre@freebsd.org) Received: (qmail 19697 invoked from network); 22 Jun 2009 22:15:25 -0000 Received: from localhost (HELO [127.0.0.1]) ([127.0.0.1]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 22 Jun 2009 22:15:25 -0000 Message-ID: <4A40056F.4000207@freebsd.org> Date: Tue, 23 Jun 2009 00:27:59 +0200 From: Andre Oppermann User-Agent: Thunderbird 1.5.0.14 (Windows/20071210) MIME-Version: 1.0 To: Kip Macy References: <200906221935.n5MJZdLJ050266@svn.freebsd.org> <3c1674c90906221510s9b62e25w3a5d41c21bd512e1@mail.gmail.com> In-Reply-To: <3c1674c90906221510s9b62e25w3a5d41c21bd512e1@mail.gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Sam Leffler Subject: Re: svn commit: r194643 - head/sys/kern X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Jun 2009 22:54:44 -0000 Kip Macy wrote: > On Mon, Jun 22, 2009 at 12:35 PM, Andre Oppermann wrote: >> Author: andre >> Date: Mon Jun 22 19:35:39 2009 >> New Revision: 194643 >> URL: http://svn.freebsd.org/changeset/base/194643 >> >> Log: >> Update m_demote: >> - remove HT_HEADER test (MT_HEADER == MT_DATA for some time now) >> - be more pedantic about m_nextpkt in other than first mbuf >> - update m_flags to be retained >> >> Modified: >> head/sys/kern/uipc_mbuf.c >> >> Modified: head/sys/kern/uipc_mbuf.c >> ============================================================================== >> --- head/sys/kern/uipc_mbuf.c Mon Jun 22 19:09:48 2009 (r194642) >> +++ head/sys/kern/uipc_mbuf.c Mon Jun 22 19:35:39 2009 (r194643) >> @@ -320,11 +320,13 @@ m_demote(struct mbuf *m0, int all) >> m->m_flags &= ~M_PKTHDR; >> bzero(&m->m_pkthdr, sizeof(struct pkthdr)); >> } >> - if (m->m_type == MT_HEADER) >> - m->m_type = MT_DATA; >> - if (m != m0 && m->m_nextpkt != NULL) >> + if (m != m0 && m->m_nextpkt != NULL) { >> + KASSERT(m->m_nextpkt == NULL, >> + ("%s: m_nextpkt not NULL", __func__)); >> + m_freem(m->m_nextpkt); >> m->m_nextpkt = NULL; >> - m->m_flags = m->m_flags & (M_EXT|M_EOR|M_RDONLY|M_FREELIST); >> + } >> + m->m_flags = m->m_flags & (M_EXT|M_RDONLY|M_FREELIST|M_NOFREE); >> } >> > > > Freeing an mbuf that shouldn't be there is not a safe change. You > don't know that m_nextpkt isn't pointing at some random value in > memory and that doing so isn't going to lead to some inexplicable > crash some time later. This is not a good strategy from a support > standpoint. If m_nextpkt is a random value we will crash anyway at the next place where it is being looked at. If it is not being looked at we just get by by chance. If m_nextpkt is valid we either yank it from someone else who will crash later on or if we just NULL it out we may have a memory leak. IMHO the latter is worse as a slowly building memory leak is much harder to track down than a crash which provides kernel dumps and backtraces. As soon as INVARIANTS are enable the KASSERT will catch the offender red handed. This is my opinion on it. Other views are welcome. -- Andre