Review needed: Mbuf double-free detection patch

[Mike Silbersack]

I'd be interested in comments on the attached patch from anyone who's been
doing work with network drivers & such.  All it does is add a M_FREELIST
flag which is set whenever a mbuf is freed.  If m_free or m_freem find
this flag to be set, they will panic, as this is a clear sign that the
mbuf was freed twice.  (All flags are cleared whenever a mbuf is
taken off the freelist, so false M_FREELIST hits shouldn't occur.)

The system isn't perfect, as it won't catch mbufs which are reallocated
before their second free occurs.  However, it does seem to do a good job
in catching simple double-free errors, which previously caused corruption
that lead to panics in codepaths totally unrelated to the original
double-free.  (One of my double-free tests without this code managed to
cause a mutex-related panic, somehow!)

I could probably make this code test for use-after-free by checksumming
the entire mbuf when M_FREELIST is set and verifying that the checksum has
not changed when the mbuf is reallocated, but I think this code is useful
enough as it is.

Comments?

Thanks,

Mike "Silby" Silbersack
-------------- next part --------------
diff -u -r /usr/src/sys.old/kern/subr_mbuf.c /usr/src/sys/kern/subr_mbuf.c
--- /usr/src/sys.old/kern/subr_mbuf.c	Wed Apr 30 00:05:03 2003
+++ /usr/src/sys/kern/subr_mbuf.c	Wed Apr 30 14:28:31 2003
@@ -1380,6 +1380,9 @@
 	int cchnum;
 	short persist = 0;
 
+	if (mb->m_flags & M_FREELIST)
+		panic("m_free detected a mbuf double-free");
+	mb->m_flags |= M_FREELIST;
 	if ((mb->m_flags & M_PKTHDR) != 0)
 		m_tag_delete_chain(mb, NULL);
 	nb = mb->m_next;
@@ -1422,6 +1425,9 @@
 	short persist;
 
 	while (mb != NULL) {
+		if (mb->m_flags & M_FREELIST)
+			panic("m_freem detected a mbuf double-free");
+		mb->m_flags |= M_FREELIST;
 		if ((mb->m_flags & M_PKTHDR) != 0)
 			m_tag_delete_chain(mb, NULL);
 		persist = 0;
diff -u -r /usr/src/sys.old/sys/mbuf.h /usr/src/sys/sys/mbuf.h
--- /usr/src/sys.old/sys/mbuf.h	Wed Apr 30 00:04:00 2003
+++ /usr/src/sys/sys/mbuf.h	Wed Apr 30 12:49:52 2003
@@ -153,6 +153,7 @@
 #define	M_PROTO3	0x0040	/* protocol-specific */
 #define	M_PROTO4	0x0080	/* protocol-specific */
 #define	M_PROTO5	0x0100	/* protocol-specific */
+#define M_FREELIST	0x4000	/* mbuf is on the free list */
 
 /*
  * mbuf pkthdr flags (also stored in m_flags).