From nobody Mon Jan 29 14:28:18 2024 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 4TNrJ271Yrz58nwb; Mon, 29 Jan 2024 14:28:18 +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 4TNrJ25Ybjz4Wbf; Mon, 29 Jan 2024 14:28:18 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1706538498; 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=gr/szfyg2xKmEm6B0WaKT5s0v6PBfjXDhLHQH8awvdQ=; b=EUZhmyZdMYm1RCj9XtX0gj7zGWC2k0CEDhA8ZpszlNRNHaS4bBC2lYOlVCEfQvl5mulvxY JwnfEn6LKcIh41t2OOx6l+2DqYtP+nWa4drFL+3hpUCARKUFaA0NvIfVNgnoj4JYlYXo+9 KIttKj5EvraqTIEFjh9oHqzbV/BOkC9Y0h2qwITj4ir8o3ItLjrlfC+fleyaWubZIX6bqN drMgbLLlqZQWYFqk5/xeDKIEvZr95Kk2FIW5Z8PirxLgLsbo1x9LrwBBH6ev/mZ0uehxxf RZEx+jYCPkIHvlcfO+IDqbLMDKgN8YAAnY+W3ZK+iPK1dwpVnppAGBs505rjOA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1706538498; 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=gr/szfyg2xKmEm6B0WaKT5s0v6PBfjXDhLHQH8awvdQ=; b=wuCf1U2aQgPbg49nFdrrKUF0g/qWRSUSCYK6Ugqsykd9L2Af/XYA0AyFpH0Yd2vaFq1JUx I86VHJ/wGozCbDdEWDkYd1zUar9lUjI4HcG7BYFWPjy3NHaklii7A3wnlc78agkL1HDxjS B2wHpQzsIy07AZBA94M03dpFojCOiCUjnWhR6u3vUCf5M5SssR00fhLtlQqBO0HSIua4s9 Y+sLzo9b8mYocWFmvGKQ2BIN2sPxqp+s4pgptmkfFaXAIb7cHka3T4+xUAMqUFBjATo55W /MjIHTeIjGKeG15L3be0EhQebTRvfxLGFMecqAP+rgxqCM7InPhuogpPefgSCA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1706538498; a=rsa-sha256; cv=none; b=Iu9JEZm+yzigG5QugF4xmhge/ooSNlFvdiEZhkp5ezWsRkvTlrshE3UJMVWsHrmgvGJDvS CE4lx88SV7mKpEmtBJ+vOf1cVok0MN+DMgrDy3ZIH4De5zb04o3pAhcUTp6+qaRH26RXsy VHcgN8/6ucViGVBrKMzobMzW/XxSgbAwFkVXl+/UMRxdFnMosGWIkD2K24fTYh/GlcO9G1 c9WaT1qbUlS5wG/OUkmKRk76FxzUC+JQnUViN97Mk3Rv7tqfc/sJWDGL9j6hR8ciRzFeRg oqtKRPtknhX9aU4u8iQBSj+6aZSvxWujakiQQAGjRh4p29LXTH+KSOALcsuiBQ== 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 4TNrJ24g76ztw1; Mon, 29 Jan 2024 14:28:18 +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 40TESIM4086933; Mon, 29 Jan 2024 14:28:18 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 40TESIjk086930; Mon, 29 Jan 2024 14:28:18 GMT (envelope-from git) Date: Mon, 29 Jan 2024 14:28:18 GMT Message-Id: <202401291428.40TESIjk086930@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: dcaf7895039d - stable/13 - condvar: Fix a user-after-free in _cv_wait() when ktrace is enabled 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: markj X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: dcaf7895039d3f71fc4ab9da1697c126c7d6bf44 Auto-Submitted: auto-generated The branch stable/13 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=dcaf7895039d3f71fc4ab9da1697c126c7d6bf44 commit dcaf7895039d3f71fc4ab9da1697c126c7d6bf44 Author: Mark Johnston AuthorDate: 2024-01-15 17:29:02 +0000 Commit: Mark Johnston CommitDate: 2024-01-29 14:26:29 +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 (cherry picked from commit a5ef95cd228e43bcc459a5c8a9911e57888ba5fd) --- 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) {