The branch main has been updated by mav:
URL:
https://cgit.FreeBSD.org/src/commit/?id=a03c23931eec567b0957c2a0b1102dba8d538d98
commit a03c23931eec567b0957c2a0b1102dba8d538d98
Author: Alexander Motin <m...@freebsd.org>
AuthorDate: 2023-11-10 00:46:26 +0000
Commit: Alexander Motin <m...@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)
