Date: Wed, 30 Apr 2003 14:35:23 -0500 (CDT) From: Mike Silbersack <silby@silby.com> To: freebsd-net@freebsd.org Subject: Review needed: Mbuf double-free detection patch Message-ID: <20030430142532.F3741@odysseus.silby.com>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
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
[-- Attachment #2 --]
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).
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030430142532.F3741>