From d52718a5f80de8a8b7813936e7a9078c56b6bbc7 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Thu, 16 Oct 2008 21:40:32 +0000 Subject: [PATCH] Fixed serious race condition: If the thread waiting for a request was interrupted, another thread closing the other end of the pipe could invoke thread_unblock() while the first thread already entered mutex_lock(). This would make the first thread think it successfully locked the mutex, without removing its (on-stack) wait entry from the mutex queue, thus leading to crashes. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@28195 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/system/kernel/fs/fifo.cpp | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/system/kernel/fs/fifo.cpp b/src/system/kernel/fs/fifo.cpp index 4540ec4716..c7d8fdad10 100644 --- a/src/system/kernel/fs/fifo.cpp +++ b/src/system/kernel/fs/fifo.cpp @@ -76,23 +76,34 @@ class RingBuffer { class ReadRequest : public DoublyLinkedListLinkImpl { public: ReadRequest() - : fThread(find_thread(NULL)) + : + fThread(thread_get_current_thread()), + fNotified(true) { + B_INITIALIZE_SPINLOCK(&fLock); } - void SetUnnotified() { fNotified = false; } + void SetNotified(bool notified) + { + InterruptsSpinLocker _(fLock); + fNotified = notified; + } void Notify() { + InterruptsSpinLocker _(fLock); + if (!fNotified) { - thread_unblock(fThread, B_OK); + SpinLocker threadLocker(gThreadSpinlock); + thread_unblock_locked(fThread, B_OK); fNotified = true; } } private: - thread_id fThread; - bool fNotified; + spinlock fLock; + struct thread* fThread; + volatile bool fNotified; }; @@ -464,15 +475,20 @@ Inode::RemoveReadRequest(ReadRequest &request) status_t Inode::WaitForReadRequest(ReadRequest &request) { - request.SetUnnotified(); - // add the entry to wait on thread_prepare_to_block(thread_get_current_thread(), B_CAN_INTERRUPT, THREAD_BLOCK_TYPE_OTHER, "fifo read request"); + request.SetNotified(false); + // wait mutex_unlock(&fRequestLock); status_t status = thread_block(); + + // Before going to lock again, we need to make sure no one tries to + // unblock us. Otherwise that would screw with mutex_lock(). + request.SetNotified(true); + mutex_lock(&fRequestLock); return status;