From nobody Fri Nov 10 00:57:04 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4SRL4x1kj6z50CN2; Fri, 10 Nov 2023 00:57:05 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4SRL4w6bWFz3C94; Fri, 10 Nov 2023 00:57:04 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699577824; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=No40kC4l/4A0XNO7uONwlZpv6EtGlZE3OMxBQqz41Nk=; b=CorjdP+SHNhPRHZmfeWu9nYjGtKnpfYxfWbbWj5FEEPTVzY4MtUZfX8qyh4QvzagGBFkfj ZuNHMAsMdvlS6bO2v4Qb6VW0KuuuUNGL462Ypeuw5RP1cYeBI/F2SPYTaJ3xs1NnTQzHij 9Jyj3gcuKTnxPEQI5w48eqd7R3I6mCaodZUV6KJCJXdBAGrNZhD9RxB8J5994wH+Otwi/H opN6X31piPv/lL1ya90sgDeeZ5y5L3WZlwcmo6Ed6jWjcZ0UuqC7eSJDs06lZpufG7Hl00 6OcWL57MqSoPtjZmLBvBvrSHvLZyigTExpLGAD+k9cNDRBa0cxvJgnkSyke23Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699577824; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=No40kC4l/4A0XNO7uONwlZpv6EtGlZE3OMxBQqz41Nk=; b=J1fp0exB3YC95IW2s3HJgg7Qgp33+EUrNB9JhNvk4ho3bXXuLcAEbDJ7bCYMXVGhhY8mhr qp4KAMsox5MyDb3ZWf/fY/K2KMuA55PeZ7fzEJvXlgIhRNeFT0BVxBPKPy1/rMRVZv6lV8 kAhSGWwTL4pmxMhnxkPJtPuUtrPYIcksyN70r4V72MnUV9qwkdq5Q+3La9Ql4xZyNystUc f/FSEBuvIhZ6X2kuP/Tu7XkTnCVa90SqdwQUsVwXH2bBdw9ebyP45bQmQ6l+0BzthlL6EY /p8cGc2w1cHmlX9xxNss69YmeXrKXDmIX9DKti4gTVLzhgorOjnwmdQBnDRbDw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1699577824; a=rsa-sha256; cv=none; b=SUbEdGrw0wHZynm7ZxYuW/8O1zJsPA/D4QLVbUMl4Ddz797+Y/EAFcXZLul4DR1dpddkvS HqRrDUMXSINZsl1Gs+LP3fdFOuFoChm3yzRHEP3eqrLJBbjl5i7wrf3oxcsn2iR1K65Td3 vI+1bsSJmyxvlRvYXPTWlgdTyEDc2pOjxPm8GTJvjji6VU6MSoekTLAV1LwBYWIWH+CS66 a2Ct4ah0nWMSd+ziNRLqn2OEUkOF+9JOBRRQzF2znK7AcGwBjPZGyeTQiTsuTtF4pCBVUA AezZTQsgZBaHp9o+MzZiJb7+sveMj5uciDzCqdLldOd2DouXmIsOpV3UiWsDzQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4SRL4w5fq8zdPN; Fri, 10 Nov 2023 00:57:04 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 3AA0v4iY099666; Fri, 10 Nov 2023 00:57:04 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 3AA0v46k099663; Fri, 10 Nov 2023 00:57:04 GMT (envelope-from git) Date: Fri, 10 Nov 2023 00:57:04 GMT Message-Id: <202311100057.3AA0v46k099663@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Alexander Motin Subject: git: a03c23931eec - main - uma: Improve memory modified after free panic messages List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mav X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: a03c23931eec567b0957c2a0b1102dba8d538d98 Auto-Submitted: auto-generated The branch main has been updated by mav: URL: https://cgit.FreeBSD.org/src/commit/?id=a03c23931eec567b0957c2a0b1102dba8d538d98 commit a03c23931eec567b0957c2a0b1102dba8d538d98 Author: Alexander Motin AuthorDate: 2023-11-10 00:46:26 +0000 Commit: Alexander Motin 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 #include +#include + 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)