git: a03c23931eec - main - uma: Improve memory modified after free panic messages

The branch main has been updated by mav:

URL: https://cgit.FreeBSD.org/src/commit/?id=a03c23931eec567b0957c2a0b1102dba8d538d98

commit a03c23931eec567b0957c2a0b1102dba8d538d98
Author:     Alexander Motin <mav@FreeBSD.org>
AuthorDate: 2023-11-10 00:46:26 +0000
Commit:     Alexander Motin <mav@FreeBSD.org>
CommitDate: 2023-11-10 00:46:26 +0000

    uma: Improve memory modified after free panic messages
    
     - Pass zone pointer to trash_ctor() and report zone name in the panic
    message.  It may be difficult to figyre out zone just by the item size.
     - Do not pass user arguments to internal trash calls, pass thezone.
     - Report malloc type name in the same unified panic message.
     - Report corruption offset from the beginning of the items instead of
    the full pointer.  It makes panic message shorter and more readable.
---
 sys/kern/kern_malloc.c |  4 +--
 sys/kern/kern_mbuf.c   |  6 ++---
 sys/vm/uma_core.c      |  4 +--
 sys/vm/uma_dbg.c       | 67 ++++++++++++++++++++++++++++++++++----------------
 4 files changed, 53 insertions(+), 28 deletions(-)

diff --git a/sys/kern/kern_malloc.c b/sys/kern/kern_malloc.c
index 07f59f733e6a..da7408edd186 100644
--- a/sys/kern/kern_malloc.c
+++ b/sys/kern/kern_malloc.c
@@ -650,7 +650,7 @@ void *
 		size = (size & ~KMEM_ZMASK) + KMEM_ZBASE;
 	indx = kmemsize[size >> KMEM_ZSHIFT];
 	zone = kmemzones[indx].kz_zone[mtp_get_subzone(mtp)];
-	va = uma_zalloc(zone, flags);
+	va = uma_zalloc_arg(zone, zone, flags);
 	if (va != NULL) {
 		size = zone->uz_size;
 		if ((flags & M_ZERO) == 0) {
@@ -690,7 +690,7 @@ malloc_domain(size_t *sizep, int *indxp, struct malloc_type *mtp, int domain,
 		size = (size & ~KMEM_ZMASK) + KMEM_ZBASE;
 	indx = kmemsize[size >> KMEM_ZSHIFT];
 	zone = kmemzones[indx].kz_zone[mtp_get_subzone(mtp)];
-	va = uma_zalloc_domain(zone, NULL, domain, flags);
+	va = uma_zalloc_domain(zone, zone, domain, flags);
 	if (va != NULL)
 		*sizep = zone->uz_size;
 	*indxp = indx;
diff --git a/sys/kern/kern_mbuf.c b/sys/kern/kern_mbuf.c
index 0897ac4cc2db..60c638735ec4 100644
--- a/sys/kern/kern_mbuf.c
+++ b/sys/kern/kern_mbuf.c
@@ -703,7 +703,7 @@ mb_dtor_pack(void *mem, int size, void *arg)
 	KASSERT(m->m_ext.ext_size == MCLBYTES, ("%s: ext_size != MCLBYTES", __func__));
 	KASSERT(m->m_ext.ext_type == EXT_PACKET, ("%s: ext_type != EXT_PACKET", __func__));
 #if defined(INVARIANTS) && !defined(KMSAN)
-	trash_dtor(m->m_ext.ext_buf, MCLBYTES, arg);
+	trash_dtor(m->m_ext.ext_buf, MCLBYTES, zone_clust);
 #endif
 	/*
 	 * If there are processes blocked on zone_clust, waiting for pages
@@ -782,7 +782,7 @@ mb_zfini_pack(void *mem, int size)
 #endif
 	uma_zfree_arg(zone_clust, m->m_ext.ext_buf, NULL);
 #if defined(INVARIANTS) && !defined(KMSAN)
-	trash_dtor(mem, size, NULL);
+	trash_dtor(mem, size, zone_clust);
 #endif
 }
 
@@ -804,7 +804,7 @@ mb_ctor_pack(void *mem, int size, void *arg, int how)
 	MPASS((flags & M_NOFREE) == 0);
 
 #if defined(INVARIANTS) && !defined(KMSAN)
-	trash_ctor(m->m_ext.ext_buf, MCLBYTES, arg, how);
+	trash_ctor(m->m_ext.ext_buf, MCLBYTES, zone_clust, how);
 #endif
 
 	error = m_init(m, how, type, flags);
diff --git a/sys/vm/uma_core.c b/sys/vm/uma_core.c
index 661c98b272da..d185f12448ee 100644
--- a/sys/vm/uma_core.c
+++ b/sys/vm/uma_core.c
@@ -3468,7 +3468,7 @@ item_ctor(uma_zone_t zone, int uz_flags, int size, void *udata, int flags,
 	skipdbg = uma_dbg_zskip(zone, item);
 	if (!skipdbg && (uz_flags & UMA_ZFLAG_TRASH) != 0 &&
 	    zone->uz_ctor != trash_ctor)
-		trash_ctor(item, size, udata, flags);
+		trash_ctor(item, size, zone, flags);
 #endif
 
 	/* Check flags before loading ctor pointer. */
@@ -3510,7 +3510,7 @@ item_dtor(uma_zone_t zone, void *item, int size, void *udata,
 #ifdef INVARIANTS
 		if (!skipdbg && (zone->uz_flags & UMA_ZFLAG_TRASH) != 0 &&
 		    zone->uz_dtor != trash_dtor)
-			trash_dtor(item, size, udata);
+			trash_dtor(item, size, zone);
 #endif
 	}
 	kasan_mark_item_invalid(zone, item);
diff --git a/sys/vm/uma_dbg.c b/sys/vm/uma_dbg.c
index 76dd2bfde2fe..c256e62875c0 100644
--- a/sys/vm/uma_dbg.c
+++ b/sys/vm/uma_dbg.c
@@ -53,18 +53,22 @@
 #include <vm/uma_dbg.h>
 #include <vm/memguard.h>
 
+#include <machine/stack.h>
+
 static const u_long uma_junk = (u_long)0xdeadc0dedeadc0de;
 
 /*
  * Checks an item to make sure it hasn't been overwritten since it was freed,
  * prior to subsequent reallocation.
  *
- * Complies with standard ctor arg/return
+ * Complies with standard ctor arg/return.  arg should be zone pointer or NULL.
  */
 int
 trash_ctor(void *mem, int size, void *arg, int flags)
 {
+	struct uma_zone *zone = arg;
 	u_long *p = mem, *e;
+	int off;
 
 #ifdef DEBUG_MEMGUARD
 	if (is_memguard_addr(mem))
@@ -73,19 +77,22 @@ trash_ctor(void *mem, int size, void *arg, int flags)
 
 	e = p + size / sizeof(*p);
 	for (; p < e; p++) {
-		if (__predict_true(*p == uma_junk))
-			continue;
-		panic("Memory modified after free %p(%d) val=%lx @ %p\n",
-		    mem, size, *p, p);
+		if (__predict_false(*p != uma_junk))
+			goto dopanic;
 	}
 	return (0);
+
+dopanic:
+	off = (uintptr_t)p - (uintptr_t)mem;
+	panic("Memory modified after free %p (%d, %s) + %d = %lx\n",
+	    mem, size, zone ? zone->uz_name : "", off,  *p);
+	return (0);
 }
 
 /*
  * Fills an item with predictable garbage
  *
  * Complies with standard dtor arg/return
- *
  */
 void
 trash_dtor(void *mem, int size, void *arg)
@@ -106,7 +113,6 @@ trash_dtor(void *mem, int size, void *arg)
  * Fills an item with predictable garbage
  *
  * Complies with standard init arg/return
- *
  */
 int
 trash_init(void *mem, int size, int flags)
@@ -116,10 +122,10 @@ trash_init(void *mem, int size, int flags)
 }
 
 /*
- * Checks an item to make sure it hasn't been overwritten since it was freed.
+ * Checks an item to make sure it hasn't been overwritten since it was freed,
+ * prior to freeing it back to available memory.
  *
  * Complies with standard fini arg/return
- *
  */
 void
 trash_fini(void *mem, int size)
@@ -127,11 +133,19 @@ trash_fini(void *mem, int size)
 	(void)trash_ctor(mem, size, NULL, 0);
 }
 
+/*
+ * Checks an item to make sure it hasn't been overwritten since it was freed,
+ * prior to subsequent reallocation.
+ *
+ * Complies with standard ctor arg/return.  arg should be zone pointer or NULL.
+ */
 int
 mtrash_ctor(void *mem, int size, void *arg, int flags)
 {
-	struct malloc_type **ksp;
+	struct uma_zone *zone = arg;
 	u_long *p = mem, *e;
+	struct malloc_type **ksp;
+	int off, osize = size;
 
 #ifdef DEBUG_MEMGUARD
 	if (is_memguard_addr(mem))
@@ -139,17 +153,31 @@ mtrash_ctor(void *mem, int size, void *arg, int flags)
 #endif
 
 	size -= sizeof(struct malloc_type *);
-	ksp = (struct malloc_type **)mem;
-	ksp += size / sizeof(struct malloc_type *);
 
 	e = p + size / sizeof(*p);
 	for (; p < e; p++) {
-		if (__predict_true(*p == uma_junk))
-			continue;
-		printf("Memory modified after free %p(%d) val=%lx @ %p\n",
-		    mem, size, *p, p);
-		panic("Most recently used by %s\n", (*ksp == NULL)?
-		    "none" : (*ksp)->ks_shortdesc);
+		if (__predict_false(*p != uma_junk))
+			goto dopanic;
+	}
+	return (0);
+
+dopanic:
+	off = (uintptr_t)p - (uintptr_t)mem;
+	ksp = (struct malloc_type **)mem;
+	ksp += size / sizeof(struct malloc_type *);
+	if (*ksp != NULL && INKERNEL((uintptr_t)*ksp)) {
+		/*
+		 * If *ksp is corrupted we may be unable to panic clean,
+		 * so print what we have reliably while we still can.
+		 */
+		printf("Memory modified after free %p (%d, %s, %p) + %d = %lx\n",
+		    mem, osize, zone ? zone->uz_name : "", *ksp, off, *p);
+		panic("Memory modified after free %p (%d, %s, %s) + %d = %lx\n",
+		    mem, osize, zone ? zone->uz_name : "", (*ksp)->ks_shortdesc,
+		    off, *p);
+	} else {
+		panic("Memory modified after free %p (%d, %s, %p) + %d = %lx\n",
+		    mem, osize, zone ? zone->uz_name : "", *ksp, off, *p);
 	}
 	return (0);
 }
@@ -158,7 +186,6 @@ mtrash_ctor(void *mem, int size, void *arg, int flags)
  * Fills an item with predictable garbage
  *
  * Complies with standard dtor arg/return
- *
  */
 void
 mtrash_dtor(void *mem, int size, void *arg)
@@ -181,7 +208,6 @@ mtrash_dtor(void *mem, int size, void *arg)
  * Fills an item with predictable garbage
  *
  * Complies with standard init arg/return
- *
  */
 int
 mtrash_init(void *mem, int size, int flags)
@@ -206,7 +232,6 @@ mtrash_init(void *mem, int size, int flags)
  * prior to freeing it back to available memory.
  *
  * Complies with standard fini arg/return
- *
  */
 void
 mtrash_fini(void *mem, int size)