git: a5ef95cd228e - main - condvar: Fix a user-after-free in _cv_wait() when ktrace is enabled

The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=a5ef95cd228e43bcc459a5c8a9911e57888ba5fd

commit a5ef95cd228e43bcc459a5c8a9911e57888ba5fd
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-01-15 17:29:02 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
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 <sys/sleepqueue.h>
 #include <sys/resourcevar.h>
 #ifdef KTRACE
-#include <sys/uio.h>
 #include <sys/ktrace.h>
+#include <sys/uio.h>
+#include <sys/user.h>
 #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) {