kernel: Add and fix ownership checks in mutex_destroy and mutex_transfer.

* mutex_destroy() only checked wether or not there were waiters,
   not if the lock itself was presently held by another thread.
   Now we do, which should make #15015 panic much earlier instead
   of trying to use freed memory.
 * mutex_transfer_lock() and recursive_lock_transfer_lock() did
   not check that the calling thread actually owned the lock.
   Now it does, which should trigger asserts if anyone tries
   to do this.
This commit is contained in:
Augustin Cavalier
2019-05-02 16:07:39 -04:00
parent c190864083
commit c2cbf95810
2 changed files with 16 additions and 14 deletions
+2 -10
View File
@@ -138,6 +138,7 @@ extern void mutex_init(mutex* lock, const char* name);
extern void mutex_init_etc(mutex* lock, const char* name, uint32 flags);
extern void mutex_destroy(mutex* lock);
extern status_t mutex_switch_lock(mutex* from, mutex* to);
extern void mutex_transfer_lock(mutex* lock, thread_id thread);
// Unlocks "from" and locks "to" such that unlocking and starting to wait
// for the lock is atomically. I.e. if "from" guards the object "to" belongs
// to, the operation is safe as long as "from" is held while destroying
@@ -260,15 +261,6 @@ mutex_unlock(mutex* lock)
}
static inline void
mutex_transfer_lock(mutex* lock, thread_id thread)
{
#if KDEBUG
lock->holder = thread;
#endif
}
static inline void
recursive_lock_transfer_lock(recursive_lock* lock, thread_id thread)
{
@@ -276,7 +268,7 @@ recursive_lock_transfer_lock(recursive_lock* lock, thread_id thread)
panic("invalid recursion level for lock transfer!");
#if KDEBUG
lock->lock.holder = thread;
mutex_transfer_lock(&lock->lock, thread);
#else
lock->holder = thread;
#endif
+14 -4
View File
@@ -628,10 +628,9 @@ mutex_destroy(mutex* lock)
InterruptsSpinLocker locker(lock->lock);
#if KDEBUG
if (lock->waiters != NULL && thread_get_current_thread_id()
!= lock->holder) {
panic("mutex_destroy(): there are blocking threads, but caller doesn't "
"hold the lock (%p)", lock);
if (lock->holder != -1 && thread_get_current_thread_id() != lock->holder) {
panic("mutex_destroy(): the lock (%p) is held by %" B_PRId32 ", not "
"by the caller", lock, lock->holder);
if (_mutex_lock(lock, &locker) != B_OK)
return;
locker.Lock();
@@ -691,6 +690,17 @@ mutex_switch_lock(mutex* from, mutex* to)
}
void
mutex_transfer_lock(mutex* lock, thread_id thread)
{
#if KDEBUG
if (thread_get_current_thread_id() != lock->holder)
panic("mutex_transfer_lock(): current thread is not the lock holder!");
lock->holder = thread;
#endif
}
status_t
mutex_switch_from_read_lock(rw_lock* from, mutex* to)
{