From nobody Mon Jan 29 14:28:08 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 4TNrHr6TjHz58pKd; Mon, 29 Jan 2024 14:28:08 +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 4TNrHr4BtCz4W3B; Mon, 29 Jan 2024 14:28:08 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1706538488; 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=8UuhYEX25WdilLyH2TzShx3TgjQOGHwZByXLUM8ynpc=; b=Pen08UJSffv4+rJbczErps7ZJJa9PcysNUNi73RA17GNL/irzerxQCaS9+a83+PkaT/tba N88j3BK9zLUDFl0QZgvz/9f5tAYKcIXWPkeP0tcmPJO+zDeu1UZAEpE7Pz67ZBvf05LzJ3 6iLyUebviJCyhq8Gb0mwWihE4H+eUzyjFYq7lqqjJAU3jT8Phj2nRf5Q8MHYwqIwcGaed2 WnWjfBFQLD2Gmb3/MHyU0qTWF10k/lj8XUK5b36TCrX7rUwPv7LjnFaMLFSp+nKm9SNLw5 8ojTjW236I1UOuczGXeeEI3iL3t7jaBIfcmMd1jR+65PVqVR/XeRsXjdNg7xMA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1706538488; 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=8UuhYEX25WdilLyH2TzShx3TgjQOGHwZByXLUM8ynpc=; b=SQrQMffOx5nRPy45FFlqPCC2lXzQfaBO6Qokx8TplyFK6znHuLGSGmCaAqNB9BRmVn2uGB Qc6mos2m4F6wktDCzjctNEXaX4BvDR7mIAS4EMNRMU8EP8QKzP5+Gi7+Yk+SKMVXl65+E7 zViWVjiWC1aZz8rh6qAAJrud3LD9qDpMMPUaArz7bkyQipElSoEk7ka98Oy+Dn52804Oy+ dMtk4QM4YGZ17atnySvyxrTNEmZBrVBBAbEwhkd1CzEt0c+rxddMrKab/HBSMa1VM1hwaL wVn76OmI0vsl3vY1GqmFXIn25xknoYEo0/2ZZfSGUkYT3DmYanFUJnk9gmmd6A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1706538488; a=rsa-sha256; cv=none; b=QTPaRIxQgZtrjZlEPorAYwF2Y5Vl/keK1lzUUahuGJJcmaPA/VVrpV+rF+yuyQDN9cZ8Hz RdDbH5e00OcvhxeJQgESQudEXCXgCeF2yRzrk+38c9DkOas+xhW9R1TRT3kWZ6MleklZ1P 2jl4sN6WsE1yBxpwNf59IyAWcd7dptBPTb4oFfSeLhka3dGM4XjPeqeJAKQUFSMyI/PRW6 QCLhfEXuil+aIbigQaxWsLWVG57M0VkFEoAwvLyzx3JmVLRWko+j0OTnoM9M/pxa0Q+38z bTxLqsxFaRUpKE2zmThMOqLyZ4HqP331OYx/QRlq3cr4oDqe4yypYmD9Mr89Ww== 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 4TNrHr3GRBzvgB; Mon, 29 Jan 2024 14:28:08 +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 40TES8kB086717; Mon, 29 Jan 2024 14:28:08 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 40TES8Oc086714; Mon, 29 Jan 2024 14:28:08 GMT (envelope-from git) Date: Mon, 29 Jan 2024 14:28:08 GMT Message-Id: <202401291428.40TES8Oc086714@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: 11cf350c3893 - stable/14 - 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/14 X-Git-Reftype: branch X-Git-Commit: 11cf350c3893d7077dbb28b9177585e9c903007c Auto-Submitted: auto-generated The branch stable/14 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=11cf350c3893d7077dbb28b9177585e9c903007c commit 11cf350c3893d7077dbb28b9177585e9c903007c Author: Mark Johnston AuthorDate: 2024-01-15 17:29:02 +0000 Commit: Mark Johnston CommitDate: 2024-01-29 14:25:52 +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) {