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.
This commit is contained in:
Alexander Motin 2023-11-09 19:46:26 -05:00
parent 14105aae55
commit a03c23931e
4 changed files with 53 additions and 28 deletions

View file

@ -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;

View file

@ -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);

View file

@ -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);

View file

@ -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)