From b9cedb46e27463290f32193cb07d2ee4c2536520 Mon Sep 17 00:00:00 2001 From: Bruce Evans Date: Sat, 2 Jun 2018 08:38:59 +0000 Subject: [PATCH] Fix low-level locking during panics. The SCHEDULER_STOPPED() hack breaks locking generally, and mtx_trylock_*() especially. When mtx_trylock_*() returns nonzero, naive code version here trusts it to have worked. But when SCHEDULER_STOPPED() is true, mtx_trylock_*() returns 1 without doing anything. Then mtx_unlock_*() crashes especially badly attempting to unlock iff the error is detected, since mutex unlocking functions don't check SCHEDULER_STOPPED(). syscons already didn't trust mtx_trylock_spin(), but it was missing the logic to turn on sp->kdb_locked when turning off sp->mtx_locked during panics. It also used panicstr instead of SCHEDULER_LOCKED because I thought that panicstr was more fragile. They only differ for a window of lines in panic(), and in broken cases where stop_cpus_hard() in panic() didn't work. --- sys/dev/syscons/syscons.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sys/dev/syscons/syscons.c b/sys/dev/syscons/syscons.c index c8c53e71fd9..4b5d46b9be6 100644 --- a/sys/dev/syscons/syscons.c +++ b/sys/dev/syscons/syscons.c @@ -1807,13 +1807,19 @@ sccnscrlock(sc_softc_t *sc, struct sc_cnstate *sp) * enough to ignore the protection even in the kdb_active case. */ if (kdb_active) { - sp->kdb_locked = sc->video_mtx.mtx_lock == MTX_UNOWNED || panicstr; + sp->kdb_locked = sc->video_mtx.mtx_lock == MTX_UNOWNED || + SCHEDULER_STOPPED(); sp->mtx_locked = FALSE; } else { sp->kdb_locked = FALSE; for (retries = 0; retries < 1000; retries++) { sp->mtx_locked = mtx_trylock_spin_flags(&sc->video_mtx, - MTX_QUIET) != 0 || panicstr; + MTX_QUIET) != 0; + if (SCHEDULER_STOPPED()) { + sp->kdb_locked = TRUE; + sp->mtx_locked = FALSE; + break; + } if (sp->mtx_locked) break; DELAY(1);