Kip Macy wrote:
On Mon, Jun 22, 2009 at 12:35 PM, Andre Oppermann<an...@freebsd.org> 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

_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to