From nobody Mon Jan 15 18:07:59 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 4TDKqz49m7z57FKj; Mon, 15 Jan 2024 18:07:59 +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 4TDKqz3GFzz4Jkb; Mon, 15 Jan 2024 18:07:59 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1705342079; 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=s3qsdh9Drj03qle34T5c8E8lbKszpY3AldrzIYfWcS8=; b=je+BAX3qfxLWwPzbOruV506JxquwE7LTeDSk6lMGvs7PXfqNChbSRRzN6cR0SdNNmzfwV/ E4FOIG249ZhL6k6jeEcaVyGD5z8ytimQP9V8/OpzZbeKaIxLwrmD+Dl5LByg03fnW9vk97 wGR/ORwsY9D6bEEW+ubD32inCMuBy4EX/5BlMnOouwAR9GLgDP6Bn2OHAStKU9RwJi2gTy qwacjzHKtw//PXOGq5NPVeohGGlc8sJnbqv0RrT+Ntu3uFLg919jSAG8zaOryj+N185Koi DI0t3WGTiFf9Nr6VtYn33iu0yZdMI1B5a2Lv8zBsm924OQrOwRCWnx963zIaBQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1705342079; 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=s3qsdh9Drj03qle34T5c8E8lbKszpY3AldrzIYfWcS8=; b=ZIvK9+GiWYJgy9kU/jOTrsrgm1kGiwXrqqlwKHh5qp7+P6k02zfs+dyDzgjkkBHDFR+z2a 2uOroq4snMTYvcovVSxAttMOo5CEDg+OKdc1W2T5XWmpiNRppR7gKpADf64562/DgqbAnR HskhyxQNuVnQP1TbbhWjV7EB1mVlW/gu5ygokXcMduH5CC/bQ61v9JNsvXXwDcaHaOeIoX ohkafPs7v9YUop/krZ05hI8inTV+Iw92FH1v0dnp0ota9gooQUFppY/4Uzwguv9PFBbuLF AIXzknJiaGgW8wy9ahxgn00s1aWHz7r/UWJOSbT4b+/6d7BVJ2Bs0iRG7BX8Ig== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1705342079; a=rsa-sha256; cv=none; b=QRE+9tCJMH4i1mpQXnRInKT1D081tbiy7BN9XKygRKJe5agk9FWDBOJSL8KS31JXoctfJz L12TKKje5IVRjdMs4rgtrbpEynBKIgnmBTOhBhmOCxNgFE1O4f1gBWlmowq4G7v3SMYt9b PnKdBmyhu189pIIyTAPy/nLnWXtkurQchW4kJNRoqsImfsts2py9aFMfCBIbHkyqBNBcFV CRKDbY3Ax+3SM/CSqrYQdkXuw8zHq54J+wYUyAbf7e0ufD+MmaKdDHmlYYAIF9Je0jxjIN sOk1pPLhJ5gDdNyKwZFEB1krH0/5LiJw1DqCXZzohGpKmKZUfNMNbZrb8mIqdQ== 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 4TDKqz22qwz16DY; Mon, 15 Jan 2024 18:07:59 +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 40FI7xtR080337; Mon, 15 Jan 2024 18:07:59 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 40FI7xhp080334; Mon, 15 Jan 2024 18:07:59 GMT (envelope-from git) Date: Mon, 15 Jan 2024 18:07:59 GMT Message-Id: <202401151807.40FI7xhp080334@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: a5ef95cd228e - main - condvar: Fix a user-after-free in _cv_wait() when ktrace is enabled 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: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: a5ef95cd228e43bcc459a5c8a9911e57888ba5fd Auto-Submitted: auto-generated The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=a5ef95cd228e43bcc459a5c8a9911e57888ba5fd commit a5ef95cd228e43bcc459a5c8a9911e57888ba5fd Author: Mark Johnston AuthorDate: 2024-01-15 17:29:02 +0000 Commit: Mark Johnston CommitDate: 2024-01-15 17:29:02 +0000 condvar: Fix a user-after-free in _cv_wait() when ktrace is enabled When a thread wakes up after sleeping on a CV, it must not dereference the CV structure, as it may already have been freed. At least ZFS relies on this invariant, see commit c636f94bd2ff15be5b904939872b4bce31456c18 for example. Thus, when logging context-switch events, copy the wmesg into a stack buffer while it is still safe to do so, and log that after waking up. While here, move the initial ktrcsw() call later, after assertions and the SCHEDULER_STOPPED_TD() condition are checked. Reported by: syzkaller Reviewed by: kib MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D43450 --- sys/kern/kern_condvar.c | 109 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 75 insertions(+), 34 deletions(-) diff --git a/sys/kern/kern_condvar.c b/sys/kern/kern_condvar.c index 6b5f2933e041..2731f581a29f 100644 --- a/sys/kern/kern_condvar.c +++ b/sys/kern/kern_condvar.c @@ -43,8 +43,9 @@ #include #include #ifdef KTRACE -#include #include +#include +#include #endif /* @@ -107,24 +108,32 @@ void _cv_wait(struct cv *cvp, struct lock_object *lock) { WITNESS_SAVE_DECL(lock_witness); +#ifdef KTRACE + char wmesg[WMESGLEN + 1]; +#endif struct lock_class *class; struct thread *td; uintptr_t lock_state; td = curthread; - lock_state = 0; -#ifdef KTRACE - if (KTRPOINT(td, KTR_CSW)) - ktrcsw(1, 0, cv_wmesg(cvp)); -#endif CV_ASSERT(cvp, lock, td); WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, lock, "Waiting on \"%s\"", cvp->cv_description); - class = LOCK_CLASS(lock); if (SCHEDULER_STOPPED_TD(td)) return; +#ifdef KTRACE + if (KTRPOINT(td, KTR_CSW)) { + strlcpy(wmesg, cv_wmesg(cvp), sizeof(wmesg)); + ktrcsw(1, 0, wmesg); + } else { + wmesg[0] = '\0'; + } +#endif + + class = LOCK_CLASS(lock); + lock_state = 0; sleepq_lock(cvp); CV_WAITERS_INC(cvp); @@ -145,7 +154,7 @@ _cv_wait(struct cv *cvp, struct lock_object *lock) #ifdef KTRACE if (KTRPOINT(td, KTR_CSW)) - ktrcsw(0, 0, cv_wmesg(cvp)); + ktrcsw(0, 0, wmesg); #endif PICKUP_GIANT(); if (lock != &Giant.lock_object) { @@ -161,14 +170,13 @@ _cv_wait(struct cv *cvp, struct lock_object *lock) void _cv_wait_unlock(struct cv *cvp, struct lock_object *lock) { +#ifdef KTRACE + char wmesg[WMESGLEN + 1]; +#endif struct lock_class *class; struct thread *td; td = curthread; -#ifdef KTRACE - if (KTRPOINT(td, KTR_CSW)) - ktrcsw(1, 0, cv_wmesg(cvp)); -#endif CV_ASSERT(cvp, lock, td); WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, lock, "Waiting on \"%s\"", cvp->cv_description); @@ -181,6 +189,15 @@ _cv_wait_unlock(struct cv *cvp, struct lock_object *lock) return; } +#ifdef KTRACE + if (KTRPOINT(td, KTR_CSW)) { + strlcpy(wmesg, cv_wmesg(cvp), sizeof(wmesg)); + ktrcsw(1, 0, wmesg); + } else { + wmesg[0] = '\0'; + } +#endif + sleepq_lock(cvp); CV_WAITERS_INC(cvp); @@ -196,7 +213,7 @@ _cv_wait_unlock(struct cv *cvp, struct lock_object *lock) #ifdef KTRACE if (KTRPOINT(td, KTR_CSW)) - ktrcsw(0, 0, cv_wmesg(cvp)); + ktrcsw(0, 0, wmesg); #endif PICKUP_GIANT(); } @@ -211,25 +228,33 @@ int _cv_wait_sig(struct cv *cvp, struct lock_object *lock) { WITNESS_SAVE_DECL(lock_witness); +#ifdef KTRACE + char wmesg[WMESGLEN + 1]; +#endif struct lock_class *class; struct thread *td; uintptr_t lock_state; int rval; td = curthread; - lock_state = 0; -#ifdef KTRACE - if (KTRPOINT(td, KTR_CSW)) - ktrcsw(1, 0, cv_wmesg(cvp)); -#endif CV_ASSERT(cvp, lock, td); WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, lock, "Waiting on \"%s\"", cvp->cv_description); - class = LOCK_CLASS(lock); if (SCHEDULER_STOPPED_TD(td)) return (0); +#ifdef KTRACE + if (KTRPOINT(td, KTR_CSW)) { + strlcpy(wmesg, cv_wmesg(cvp), sizeof(wmesg)); + ktrcsw(1, 0, wmesg); + } else { + wmesg[0] = '\0'; + } +#endif + + class = LOCK_CLASS(lock); + lock_state = 0; sleepq_lock(cvp); CV_WAITERS_INC(cvp); @@ -251,7 +276,7 @@ _cv_wait_sig(struct cv *cvp, struct lock_object *lock) #ifdef KTRACE if (KTRPOINT(td, KTR_CSW)) - ktrcsw(0, 0, cv_wmesg(cvp)); + ktrcsw(0, 0, wmesg); #endif PICKUP_GIANT(); if (lock != &Giant.lock_object) { @@ -272,24 +297,32 @@ _cv_timedwait_sbt(struct cv *cvp, struct lock_object *lock, sbintime_t sbt, sbintime_t pr, int flags) { WITNESS_SAVE_DECL(lock_witness); +#ifdef KTRACE + char wmesg[WMESGLEN + 1]; +#endif struct lock_class *class; struct thread *td; int lock_state, rval; td = curthread; - lock_state = 0; -#ifdef KTRACE - if (KTRPOINT(td, KTR_CSW)) - ktrcsw(1, 0, cv_wmesg(cvp)); -#endif CV_ASSERT(cvp, lock, td); WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, lock, "Waiting on \"%s\"", cvp->cv_description); - class = LOCK_CLASS(lock); if (SCHEDULER_STOPPED_TD(td)) return (0); +#ifdef KTRACE + if (KTRPOINT(td, KTR_CSW)) { + strlcpy(wmesg, cv_wmesg(cvp), sizeof(wmesg)); + ktrcsw(1, 0, wmesg); + } else { + wmesg[0] = '\0'; + } +#endif + + class = LOCK_CLASS(lock); + lock_state = 0; sleepq_lock(cvp); CV_WAITERS_INC(cvp); @@ -311,7 +344,7 @@ _cv_timedwait_sbt(struct cv *cvp, struct lock_object *lock, sbintime_t sbt, #ifdef KTRACE if (KTRPOINT(td, KTR_CSW)) - ktrcsw(0, 0, cv_wmesg(cvp)); + ktrcsw(0, 0, wmesg); #endif PICKUP_GIANT(); if (lock != &Giant.lock_object) { @@ -334,24 +367,32 @@ _cv_timedwait_sig_sbt(struct cv *cvp, struct lock_object *lock, sbintime_t sbt, sbintime_t pr, int flags) { WITNESS_SAVE_DECL(lock_witness); +#ifdef KTRACE + char wmesg[WMESGLEN + 1]; +#endif struct lock_class *class; struct thread *td; int lock_state, rval; td = curthread; - lock_state = 0; -#ifdef KTRACE - if (KTRPOINT(td, KTR_CSW)) - ktrcsw(1, 0, cv_wmesg(cvp)); -#endif CV_ASSERT(cvp, lock, td); WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, lock, "Waiting on \"%s\"", cvp->cv_description); - class = LOCK_CLASS(lock); if (SCHEDULER_STOPPED_TD(td)) return (0); +#ifdef KTRACE + if (KTRPOINT(td, KTR_CSW)) { + strlcpy(wmesg, cv_wmesg(cvp), sizeof(wmesg)); + ktrcsw(1, 0, wmesg); + } else { + wmesg[0] = '\0'; + } +#endif + + class = LOCK_CLASS(lock); + lock_state = 0; sleepq_lock(cvp); CV_WAITERS_INC(cvp); @@ -374,7 +415,7 @@ _cv_timedwait_sig_sbt(struct cv *cvp, struct lock_object *lock, #ifdef KTRACE if (KTRPOINT(td, KTR_CSW)) - ktrcsw(0, 0, cv_wmesg(cvp)); + ktrcsw(0, 0, wmesg); #endif PICKUP_GIANT(); if (lock != &Giant.lock_object) {