Mbuf double-free guilty party detection patch

Mike Silbersack silby at silby.com
Sat Jun 25 02:31:46 GMT 2005


The attached patch stores the address of who freed an 
mbuf/cluster/whatever inside it, then prints that address when panicing. 
You can then feed that address into "x 0xwhatever" in DDB to see who the 
semi-guilty party is.

Two flaws in the patch as is:

- It's messy and not compatible with non-i386, cleanups are needed.

- If the mbuf in question is part of a mbuf chain, we'll see m_freem as 
the guilty party, because it called m_free.

So, if you're one of the people seeing panics due to mbufs being used 
after free, please try applying the patch and see what results you get. 
If you keep getting m_freem as the previous user, then I'll have to 
enhance it to work around that.

Mike "Silby" Silbersack
-------------- next part --------------
diff -u -r /usr/src/sys.old/kern/uipc_mbuf.c /usr/src/sys/kern/uipc_mbuf.c
--- /usr/src/sys.old/kern/uipc_mbuf.c	Fri Jun 24 20:13:59 2005
+++ /usr/src/sys/kern/uipc_mbuf.c	Fri Jun 24 20:50:16 2005
@@ -219,7 +219,7 @@
  * storage attached to them if the reference count hits 0.
  */
 void
-mb_free_ext(struct mbuf *m)
+mb_free_ext(struct mbuf *m, void *arg)
 {
 	u_int cnt;
 	int dofree;
@@ -249,10 +249,10 @@
 		 * Do the free, should be safe.
 		 */
 		if (m->m_ext.ext_type == EXT_PACKET) {
-			uma_zfree(zone_pack, m);
+			uma_zfree_arg(zone_pack, m, arg);
 			return;
 		} else if (m->m_ext.ext_type == EXT_CLUSTER) {
-			uma_zfree(zone_clust, m->m_ext.ext_buf);
+			uma_zfree_arg(zone_clust, m->m_ext.ext_buf, arg);
 			m->m_ext.ext_buf = NULL;
 		} else {
 			(*(m->m_ext.ext_free))(m->m_ext.ext_buf,
@@ -266,7 +266,7 @@
 			m->m_ext.ext_buf = NULL;
 		}
 	}
-	uma_zfree(zone_mbuf, m);
+	uma_zfree_arg(zone_mbuf, m, arg);
 }
 
 /*
@@ -1381,4 +1381,19 @@
 	if (m_final)
 		m_freem(m_final);
 	return (NULL);
+}
+
+struct mbuf *
+m_free(struct mbuf *m)
+{
+        struct mbuf *n = m->m_next;
+ 
+#ifdef INVARIANTS
+        m->m_flags |= M_FREELIST;
+#endif
+        if (m->m_flags & M_EXT)
+                mb_free_ext(m, __builtin_return_address(0));
+        else
+                uma_zfree_arg(zone_mbuf, m, __builtin_return_address(0));
+        return n;
 }
diff -u -r /usr/src/sys.old/sys/mbuf.h /usr/src/sys/sys/mbuf.h
--- /usr/src/sys.old/sys/mbuf.h	Fri Jun 24 20:17:31 2005
+++ /usr/src/sys/sys/mbuf.h	Fri Jun 24 20:53:07 2005
@@ -350,10 +350,10 @@
 static __inline struct mbuf	*m_gethdr(int how, short type);
 static __inline struct mbuf	*m_getcl(int how, short type, int flags);
 static __inline struct mbuf	*m_getclr(int how, short type);	/* XXX */
-static __inline struct mbuf	*m_free(struct mbuf *m);
+struct mbuf     *m_free(struct mbuf *m);
 static __inline void		 m_clget(struct mbuf *m, int how);
 static __inline void		 m_chtype(struct mbuf *m, short new_type);
-void				 mb_free_ext(struct mbuf *);
+void				 mb_free_ext(struct mbuf *, void *arg);
 
 static __inline
 struct mbuf *
@@ -404,7 +404,8 @@
 	return (uma_zalloc_arg(zone_pack, &args, how));
 }
 
-static __inline
+#if 0
+static
 struct mbuf *
 m_free(struct mbuf *m)
 {
@@ -414,11 +415,12 @@
 	m->m_flags |= M_FREELIST;
 #endif
 	if (m->m_flags & M_EXT)
-		mb_free_ext(m);
+		mb_free_ext(m, __builtin_return_address(0));
 	else
-		uma_zfree(zone_mbuf, m);
+		uma_zfree_arg(zone_mbuf, m, __builtin_return_address(0));
 	return n;
 }
+#endif
 
 static __inline
 void
diff -u -r /usr/src/sys.old/vm/uma_dbg.c /usr/src/sys/vm/uma_dbg.c
--- /usr/src/sys.old/vm/uma_dbg.c	Fri Jun 24 20:13:27 2005
+++ /usr/src/sys/vm/uma_dbg.c	Fri Jun 24 21:11:05 2005
@@ -66,11 +66,14 @@
 	u_int32_t *p;
 
 	cnt = size / sizeof(uma_junk);
+	cnt -= sizeof(void *);
 
 	for (p = mem; cnt > 0; cnt--, p++)
-		if (*p != uma_junk)
+		if (*p != uma_junk) {
+			printf("This memory last freed by: %p\n", (void *)*p);
 			panic("Memory modified after free %p(%d) val=%x @ %p\n",
 			    mem, size, *p, p);
+		}
 	return (0);
 }
 
@@ -87,9 +90,11 @@
 	u_int32_t *p;
 
 	cnt = size / sizeof(uma_junk);
+	cnt -= sizeof(void *);
 
 	for (p = mem; cnt > 0; cnt--, p++)
 		*p = uma_junk;
+	*p = (int)arg;
 }
 
 /*


More information about the freebsd-current mailing list