From nobody Mon Jul 15 18:21:54 2024 X-Original-To: dev-commits-src-main@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 4WN9X334rsz5RQJy; Mon, 15 Jul 2024 18:21:55 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (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 "smtp.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WN9X32dS1z4MPH; Mon, 15 Jul 2024 18:21:55 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1721067715; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jzuUbGVycZXQs86mrYXN32yYlJIWAprhEjXNy8UvMmc=; b=sUC/DiL1Kk46R4wN35AZPnBg7/ZPtjiCc72kSR7pZLcHbySLGe2O+MA6mWawV62JcVB07P pYAXst9cAxKt8ILiaJCZhE//zt3KI30LggCo/W5QDZyLg75XqatCW6cINktWT8+q6Lmtk9 9/aLzW8F0Kp7rt4DYF10NJgn099rJBzWQjczAoc4QU0AH4DmTLCJzrLjjjNwXfIvdgdJ5j wvo9YxgdPRtYczu2Cq87fhIeIPtjccIGKrQuAKy+tQy3O5PpaWqld4yciO+aFr2mc2PN2U AP/o7gpzOrCJp8n3l78s24cK0iWhfabhCEWGgfvty7rTptmZ8cUs76SDxN3Wfw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1721067715; a=rsa-sha256; cv=none; b=xT+ll4n4I+JE7l1pIioSsk8tIEw5Y6pGpcvsF3DUgvpvBtgafPyJeIGX2p/xB5Ea+W4OJL QfBNVwQjR/4PNumQSFH2OnDRVzof5Lt4teo4fDZ7aQoErEXEU+WbEb8sRMASM5iH86CpmK lVS0Vayxoavlj+Vs1lxQHIroX1bE3Az9FQYAYcu04zCZLlIliaqewei7a0hZzyrQrnaLSq Pzf3FHviPZ8izNXRWdqfkcwD1FMCG76P5lgszzoy5nyzqb4VZv9sJLpYeB2JTgQL/8PBpu 8WUd6brlRL1h9ZeUeoeZzjPPI0puhdV7KjBpS9hux+dtFdUweSuxbPkIyJPF3Q== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1721067715; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jzuUbGVycZXQs86mrYXN32yYlJIWAprhEjXNy8UvMmc=; b=kKnYGli4ACaB2fMJ3t+PhMGFx+U98xwjtP+k0drbbZ7vg+hodf/HYiDnChQXMi70q8N1+R al9etB29EEsp3bcdCQlypuAcSsTQvI2mjmDuqGH/G+yp749pmW7MbUo3xneKh+JRjFIzw1 d3eCC6CIEpInRXeswtOoCmeow8u4P/u9QsnMrNGLMz+mPuem1ZMTm1o3Z3wT4SLTW2RYqN Xwc9yxQ6O/mlEDvFIgyFPotpZgSRL5bmCTzyErns31YjIh1eiukmk9p/RMdQgXWhG3zdhn LIz0yfQsCKhZiae6dV3+GBfupSKBwk4jsFmL0P6M3O/nLZsgqpzoHTgspJ3qLA== Received: from [IPV6:2601:5c0:4200:b830:d8c4:b767:290b:ce25] (unknown [IPv6:2601:5c0:4200:b830:d8c4:b767:290b:ce25]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4WN9X30cC5z1Q9m; Mon, 15 Jul 2024 18:21:55 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: Date: Mon, 15 Jul 2024 14:21:54 -0400 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: git: 87ee63bac69d - main - locks: add a runtime check for missing turnstile Content-Language: en-US To: Mateusz Guzik Cc: Mateusz Guzik , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202407111107.46BB7uSb007077@gitrepo.freebsd.org> <81cfe7ad-cbee-4122-abef-e47ce2b34f05@FreeBSD.org> From: John Baldwin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 7/15/24 13:59, Mateusz Guzik wrote: > On Mon, Jul 15, 2024 at 6:22 PM John Baldwin wrote: >> >> On 7/11/24 07:07, Mateusz Guzik wrote: >>> The branch main has been updated by mjg: >>> >>> URL: https://cgit.FreeBSD.org/src/commit/?id=87ee63bac69dc49291f55590b8baa57cad6c7d85 >>> >>> commit 87ee63bac69dc49291f55590b8baa57cad6c7d85 >>> Author: Mateusz Guzik >>> AuthorDate: 2024-07-11 00:17:27 +0000 >>> Commit: Mateusz Guzik >>> CommitDate: 2024-07-11 11:06:52 +0000 >>> >>> locks: add a runtime check for missing turnstile >>> >>> There are sometimes bugs which result in the unlock fast path failing, >>> which in turns causes a not-helpful crash report when dereferencing a >>> NULL turnstile. Help debugging such cases by pointing out what happened >>> along with some debug. >>> >>> Sponsored by: Rubicon Communications, LLC ("Netgate") >>> --- >>> sys/kern/kern_mutex.c | 4 +++- >>> sys/kern/kern_rwlock.c | 16 ++++++++++++---- >>> 2 files changed, 15 insertions(+), 5 deletions(-) >>> >>> diff --git a/sys/kern/kern_mutex.c b/sys/kern/kern_mutex.c >>> index 90361b23c09a..0fa624cc4bb1 100644 >>> --- a/sys/kern/kern_mutex.c >>> +++ b/sys/kern/kern_mutex.c >>> @@ -1053,7 +1053,9 @@ __mtx_unlock_sleep(volatile uintptr_t *c, uintptr_t v) >>> turnstile_chain_lock(&m->lock_object); >>> _mtx_release_lock_quick(m); >>> ts = turnstile_lookup(&m->lock_object); >>> - MPASS(ts != NULL); >>> + if (__predict_false(ts == NULL)) { >>> + panic("got NULL turnstile on mutex %p v %zx", m, v); >>> + } >> >> Hmm, this is just an expanded KASSERT() but always on rather than conditional on INVARIANTS? >> >> Do you have examples of the type of bugs that cause this? (Is it unlocking a freed mutex >> or the like?) We generally hide all these types of checks under INVARIANTS rather than >> shipping them in release kernels. >> > > Use-after-free, overflow, underflow, bitflip or what have you all can > fail the fast path. > > Once that happens and the kernel crashes with a null pointer deref, > here is a crash at netgate which prodded this: > calltrap() at calltrap+0x8/frame 0xfffffe0106720920 > --- trap 0xc, rip = 0xffffffff80d5ab70, rsp = 0xfffffe01067209f0, rbp > = 0xfffffe0106720a00 --- > turnstile_broadcast() at turnstile_broadcast+0x40/frame 0xfffffe0106720a00 > __rw_wunlock_hard() at __rw_wunlock_hard+0x9e/frame 0xfffffe0106720a30 > nd6_resolve_slow() at nd6_resolve_slow+0x2d7/frame 0xfffffe0106720aa0 > nd6_resolve() at nd6_resolve+0x125/frame 0xfffffe0106720b10 > ether_output() at ether_output+0x4e7/frame 0xfffffe0106720ba0 > ip_output_send() at ip_output_send+0xdc/frame 0xfffffe0106720be0 > ip_output() at ip_output+0x1295/frame 0xfffffe0106720ce0 > ip_forward() at ip_forward+0x3c2/frame 0xfffffe0106720d90 > ip_input() at ip_input+0x705/frame 0xfffffe0106720df0 > swi_net() at swi_net+0x138/frame 0xfffffe0106720e60 > ithread_loop() at ithread_loop+0x257/frame 0xfffffe0106720ef0 > fork_exit() at fork_exit+0x7f/frame 0xfffffe0106720f30 > fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe0106720f30 > --- trap 0, rip = 0, rsp = 0, rbp = 0 --- > > Neither the register dump nor anything in the backtrace indicate what happened. > > Since the kernel is going down anyway, one may as well get some debug from it. If you don't mind the extra branches for sanity checks, why not just run with INVARIANTS? That is, what makes these particular assertions different from other assertions such that they should be on unconditionally? The last line below applies to pretty much every other assertion in the tree. -- John Baldwin