mirror/freebsd
1
0
Fork 0
mirror of https://git.FreeBSD.org/src.git synced 2025-01-22 15:47:37 +00:00
Code Issues Releases Activity

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

Browse Source 
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
c636f94bd2 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
This commit is contained in:
Mark Johnston 2024-01-15 12:29:02 -05:00
parent 61b0092be7
commit a5ef95cd22
1 changed files with 75 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

109
sys/kern/kern_condvar.c
View File

@ -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) {