Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 15 Jul 2024 12:22:43 -0400
From:      John Baldwin <jhb@FreeBSD.org>
To:        Mateusz Guzik <mjg@FreeBSD.org>, src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   Re: git: 87ee63bac69d - main - locks: add a runtime check for missing turnstile
Message-ID:  <81cfe7ad-cbee-4122-abef-e47ce2b34f05@FreeBSD.org>
In-Reply-To: <202407111107.46BB7uSb007077@gitrepo.freebsd.org>
References:  <202407111107.46BB7uSb007077@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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 <mjg@FreeBSD.org>
> AuthorDate: 2024-07-11 00:17:27 +0000
> Commit:     Mateusz Guzik <mjg@FreeBSD.org>
> 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.

-- 
John Baldwin




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?81cfe7ad-cbee-4122-abef-e47ce2b34f05>