Firefox · DOM
CVE-2026-109752
Logic Error in DOM
Overview
Moderate
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
fordom/workers/ScriptLoader.cpp |
modified | |
ifdom/workers/ScriptLoader.cpp |
modified | |
ifdom/workers/loader/CacheLoadHandler.cpp |
modified | |
nsIReferrerInfodom/workers/loader/WorkerLoadContext.h |
modified | |
nsIURIdom/workers/loader/WorkerLoadContext.h |
modified |
Files Changed
dom/workers/ScriptLoader.cppdom/workers/loader/CacheLoadHandler.cppdom/workers/loader/NetworkLoadHandler.cppdom/workers/loader/WorkerLoadContext.cppdom/workers/loader/WorkerLoadContext.h
Patch
diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp
index c27c7ecf4c7..decb34da82a 100644
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -1579,7 +1579,9 @@ void ScriptLoaderRunnable::CancelMainThread(nsresult aCancelResult) {
mCancelMainThread = Some(aCancelResult);
for (ThreadSafeRequestHandle* handle : mLoadingRequests) {
- if (handle->IsEmpty()) {
+ // Finishing an earlier request can have scheduled execution of this one,
+ // handing it over to the worker thread.
+ if (handle->ExecutionScheduled()) {
continue;
}
@@ -1623,7 +1625,7 @@ void ScriptLoaderRunnable::DispatchProcessPendingRequests() {
// unset.
auto firstItToExecute = std::find_if(
begin, end, [](const RefPtr<ThreadSafeRequestHandle>& requestHandle) {
- return !requestHandle->mExecutionScheduled;
+ return !requestHandle->ExecutionScheduled();
});
if (firstItToExecute == end) {
@@ -1636,13 +1638,13 @@ void ScriptLoaderRunnable::DispatchProcessPendingRequests() {
const auto firstItUnexecutable =
std::find_if(firstItToExecute, end,
[](RefPtr<ThreadSafeRequestHandle>& requestHandle) {
- MOZ_ASSERT(!requestHandle->IsEmpty());
+ MOZ_ASSERT(!requestHandle->ExecutionScheduled());
if (!requestHandle->Finished()) {
return true;
}
// We can execute this one.
- requestHandle->mExecutionScheduled = true;
+ requestHandle->SetExecutionScheduled();
return false;
});
diff --git a/dom/workers/loader/CacheLoadHandler.cpp b/dom/workers/loader/CacheLoadHandler.cpp
index 0416272e539..a2d45457f39 100644
--- a/dom/workers/loader/CacheLoadHandler.cpp
+++ b/dom/workers/loader/CacheLoadHandler.cpp
@@ -47,7 +47,7 @@ void CachePromiseHandler::ResolvedCallback(JSContext* aCx,
ErrorResult& aRv) {
AssertIsOnMainThread();
// skip to schedule execution if it has been scheduled already.
- if (mRequestHandle->mExecutionScheduled) {
+ if (mRequestHandle->ExecutionScheduled()) {
return;
}
WorkerLoadContext* loadContext = mRequestHandle->GetContext();
@@ -71,7 +71,7 @@ void CachePromiseHandler::RejectedCallback(JSContext* aCx,
ErrorResult& aRv) {
AssertIsOnMainThread();
// skip to schedule execution if it has been scheduled already.
- if (mRequestHandle->mExecutionScheduled) {
+ if (mRequestHandle->ExecutionScheduled()) {
return;
}
WorkerLoadContext* loadContext = mRequestHandle->GetContext();
@@ -256,19 +256,19 @@ void CacheLoadHandler::Fail(nsresult aRv) {
mFailed = true;
+ // The request may already belong to the worker thread, in which case we can
+ // only stop the read and must leave the request alone.
+ const bool executionScheduled = mRequestHandle->ExecutionScheduled();
+
if (mPump) {
- MOZ_ASSERT_IF(!mRequestHandle->IsEmpty(),
+ MOZ_ASSERT_IF(!executionScheduled,
mRequestHandle->GetContext()->mCacheStatus ==
WorkerLoadContext::ReadingFromCache);
mPump->Cancel(aRv);
mPump = nullptr;
}
- if (mRequestHandle->mExecutionScheduled) {
- return;
- }
-
- if (mRequestHandle->IsEmpty()) {
+ if (executionScheduled) {
return;
}
@@ -301,7 +301,13 @@ void CacheLoadHandler::Fail(nsresult aRv) {
void CacheLoadHandler::Load(Cache* aCache) {
AssertIsOnMainThread();
MOZ_ASSERT(aCache);
- MOZ_ASSERT(!mRequestHandle->IsEmpty());
+
+ // The load can have been cancelled, and the request handed over to the
+ // worker thread, while CacheStorage::Open() was still pending.
+ if (mRequestHandle->ExecutionScheduled()) {
+ return;
+ }
+
WorkerLoadContext* loadContext = mRequestHandle->GetContext();
nsCOMPtr<nsIURI> uri;
@@ -343,10 +349,9 @@ void CacheLoadHandler::RejectedCallback(JSContext* aCx,
JS::Handle<JS::Value> aValue,
ErrorResult& aRv) {
AssertIsOnMainThread();
- MOZ_ASSERT(!mRequestHandle->IsEmpty());
-
- MOZ_ASSERT(mRequestHandle->GetContext()->mCacheStatus ==
- WorkerLoadContext::Uncached);
+ MOZ_ASSERT_IF(!mRequestHandle->ExecutionScheduled(),
+ mRequestHandle->GetContext()->mCacheStatus ==
+ WorkerLoadContext::Uncached);
Fail(NS_ERROR_FAILURE);
}
@@ -354,15 +359,16 @@ void CacheLoadHandler::ResolvedCallback(JSContext* aCx,
JS::Handle<JS::Value> aValue,
ErrorResult& aRv) {
AssertIsOnMainThread();
- MOZ_ASSERT(!mRequestHandle->IsEmpty());
- WorkerLoadContext* loadContext = mRequestHandle->GetContext();
// If we have already called 'Fail', we should not proceed. If we cancelled,
- // we should similarily not proceed.
- if (mFailed) {
+ // we should similarily not proceed. The request may also have been handed
+ // over to the worker thread while Cache::Match() was pending.
+ if (mFailed || mRequestHandle->ExecutionScheduled()) {
return;
}
+ WorkerLoadContext* loadContext = mRequestHandle->GetContext();
+
MOZ_ASSERT(loadContext->mCacheStatus == WorkerLoadContext::Uncached);
nsresult rv;
@@ -504,9 +510,16 @@ CacheLoadHandler::OnStreamComplete(nsIStreamLoader* aLoader,
uint32_t aStringLen,
const uint8_t* aString) {
AssertIsOnMainThread();
- if (mRequestHandle->IsEmpty()) {
+
+ // The load can be finished, and the request handed over to the worker
+ // thread, before the stream we are reading from the cache completes; see
+ // ScriptLoaderRunnable::CancelMainThread(). From then on the request belongs
+ // to the worker thread and we must not touch it.
+ if (mRequestHandle->ExecutionScheduled()) {
+ mPump = nullptr;
return NS_OK;
}
+
WorkerLoadContext* loadContext = mRequestHandle->GetContext();
mPump = nullptr;
@@ -539,7 +552,7 @@ nsresult CacheLoadHandler::DataReceivedFromCache(
const nsACString& aReferrerPolicyHeaderValue,
const nsACString& aReportingEndpointsHeaderValue) {
AssertIsOnMainThread();
- if (mRequestHandle->IsEmpty()) {
+ if (mRequestHandle->ExecutionScheduled()) {
return NS_OK;
}
WorkerLoadContext* loadContext = mRequestHandle->GetContext();
@@ -641,7 +654,7 @@ nsresult CacheLoadHandler::DataReceivedFromCache(
}
nsresult CacheLoadHandler::DataReceived() {
- MOZ_ASSERT(!mRequestHandle->IsEmpty());
+ MOZ_ASSERT(!mRequestHandle->ExecutionScheduled());
WorkerLoadContext* loadContext = mRequestHandle->GetContext();
if (loadContext->IsTopLevel()) {
diff --git a/dom/workers/loader/NetworkLoadHandler.cpp b/dom/workers/loader/NetworkLoadHandler.cpp
index 1944d74a93f..a388afa7308 100644
--- a/dom/workers/loader/NetworkLoadHandler.cpp
+++ b/dom/workers/loader/NetworkLoadHandler.cpp
@@ -53,9 +53,13 @@ NetworkLoadHandler::OnStreamComplete(nsIStreamLoader* aLoader,
nsISupports* aContext, nsresult aStatus,
uint32_t aStringLen,
const uint8_t* aString) {
- // If we have cancelled, or we have no mRequest, it means that the loader has
- // shut down and we can exit early. If the cancel result is still NS_OK
- if (mRequestHandle->IsEmpty()) {
+ AssertIsOnMainThread();
+
+ // If the request has been handed over to the worker thread, it means that
+ // the loader has shut down and we can exit early. This happens when
+ // cancelling the channel failed and the load was finished without us; see
+ // ScriptLoaderRunnable::CancelMainThread().
+ if (mRequestHandle->ExecutionScheduled()) {
return NS_OK;
}
nsresult rv = DataReceivedFromNetwork(aLoader, aStatus, aStringLen, aString);
@@ -67,7 +71,7 @@ nsresult NetworkLoadHandler::DataReceivedFromNetwork(nsIStreamLoader* aLoader,
uint32_t aStringLen,
const uint8_t* aString) {
AssertIsOnMainThread();
- MOZ_ASSERT(!mRequestHandle->IsEmpty());
+ MOZ_ASSERT(!mRequestHandle->ExecutionScheduled());
if (aStringLen > GetWorkerScriptMaxSizeInBytes()) {
Document* parentDoc = mWorkerRef->Private()->GetDocument();
@@ -337,15 +341,17 @@ NetworkLoadHandler::OnStartRequest(nsIRequest* aRequest) {
nsresult NetworkLoadHandler::PrepareForRequest(nsIRequest* aRequest) {
AssertIsOnMainThread();
- MOZ_ASSERT(!mRequestHandle->IsEmpty());
- WorkerLoadContext* loadContext = mRequestHandle->GetContext();
// If one load info cancels or hits an error, it can race with the start
- // callback coming from another load info.
- if (mRequestHandle->IsCancelled()) {
+ // callback coming from another load info. The request may even have been
+ // handed over to the worker thread already, in which case we must not touch
+ // it any more and let the caller cancel this channel.
+ if (mRequestHandle->ExecutionScheduled() || mRequestHandle->IsCancelled()) {
return NS_ERROR_FAILURE;
}
+ WorkerLoadContext* loadContext = mRequestHandle->GetContext();
+
nsCOMPtr<nsIChannel> channel = do_QueryInterface(aRequest);
// Checking the MIME type is only required for ServiceWorkers'
diff --git a/dom/workers/loader/WorkerLoadContext.cpp b/dom/workers/loader/WorkerLoadContext.cpp
index 6b807ff5976..d14552a6e48 100644
--- a/dom/workers/loader/WorkerLoadContext.cpp
+++ b/dom/workers/loader/WorkerLoadContext.cpp
@@ -31,9 +31,20 @@ ThreadSafeRequestHandle::ThreadSafeRequestHandle(
: mRequest(aRequest), mOwningEventTarget(aSyncTarget) {}
WorkerLoadContext* ThreadSafeRequestHandle::GetContext() {
+ AssertRequestNotHandedOff();
return mRequest->GetWorkerLoadContext();
}
+bool ThreadSafeRequestHandle::ExecutionScheduled() const {
+ AssertIsOnMainThread();
+ return mExecutionScheduled;
+}
+
+void ThreadSafeRequestHandle::SetExecutionScheduled() {
+ AssertIsOnMainThread();
+ mExecutionScheduled = true;
+}
+
void ThreadSafeRequestHandle::SetRunnable(
workerinternals::loader::ScriptLoaderRunnable* aRunnable) {
MutexAutoLock lock(mMutex);
@@ -124,11 +135,20 @@ ThreadSafeRequestHandle::GetCacheCreator() {
}
ThreadSafeRequestHandle::~ThreadSafeRequestHandle() {
+ // Nothing can be racing with us any more, but ReleaseRequest() may have
+ // cleared mRequest on the worker thread, so take mMutex once to make sure we
+ // observe that write rather than a stale pointer to an already freed request.
+ bool hasRequest;
+ {
+ MutexAutoLock lock(mMutex);
+ hasRequest = !!mRequest;
+ }
+
// Normally we only touch mStrongRef on the owning thread. This is safe,
// however, because when we do use mStrongRef on the owning thread we are
// always holding a strong ref to the ThreadsafeHandle via the owning
// runnable. So we cannot run the ThreadsafeHandle destructor simultaneously.
- if (!mRequest || mOwningEventTarget->IsOnCurrentThread()) {
+ if (!hasRequest || mOwningEventTarget->IsOnCurrentThread()) {
return;
}
diff --git a/dom/workers/loader/WorkerLoadContext.h b/dom/workers/loader/WorkerLoadContext.h
index ebe0a045b08..7e271911dc5 100644
--- a/dom/workers/loader/WorkerLoadContext.h
+++ b/dom/workers/loader/WorkerLoadContext.h
@@ -14,6 +14,7 @@
#include "nsIChannel.h"
#include "nsIInputStream.h"
#include "nsIRequest.h"
+#include "nsThreadUtils.h"
class nsIReferrerInfo;
class nsIURI;
@@ -176,15 +177,29 @@ class ThreadSafeRequestHandle final {
ThreadSafeRequestHandle(JS::loader::ScriptLoadRequest* aRequest,
nsISerialEventTarget* aSyncTarget);
- JS::loader::ScriptLoadRequest* GetRequest() const { return mRequest; }
+ JS::loader::ScriptLoadRequest* GetRequest() const {
+ AssertRequestNotHandedOff();
+ return mRequest;
+ }
WorkerLoadContext* GetContext();
- bool IsEmpty() { return !mRequest; }
+ bool IsEmpty() const {
+ AssertRequestNotHandedOff();
+ return !mRequest;
+ }
// Sets the owning runnable. Called on the main thread before loading begins.
void SetRunnable(workerinternals::loader::ScriptLoaderRunnable* aRunnable);
+ // Whether this handle has been handed off to a ScriptExecutorRunnable, after
+ // which the request belongs to the worker thread. Main thread only; this is
+ // the only race-free way for main-thread code to test the handoff, see the
+ // comment on mRequest.
+ bool ExecutionScheduled() const;
+
+ void SetExecutionScheduled();
+
// Runnable controls
nsresult OnStreamComplete(nsresult aStatus);
@@ -204,11 +219,15 @@ class ThreadSafeRequestHandle final {
already_AddRefed<workerinternals::loader::CacheCreator> GetCacheCreator();
- bool mExecutionScheduled = false;
-
private:
~ThreadSafeRequestHandle();
+ // Main-thread code must stop looking at the request, its WorkerLoadContext
+ // and anything else it owns once the handle has been handed off.
+ void AssertRequestNotHandedOff() const {
+ MOZ_ASSERT_IF(NS_IsMainThread(), !mExecutionScheduled);
+ }
+
// Protects mRunnable, which is read on the main thread by the accessors above
// but cleared on the worker thread by ReleaseRequest(). Without this lock the
// unsynchronized read/write races and the ScriptLoaderRunnable can be freed
@@ -217,6 +236,17 @@ class ThreadSafeRequestHandle final {
RefPtr<workerinternals::loader::ScriptLoaderRunnable> mRunnable
MOZ_GUARDED_BY(mMutex);
+ // Only ever set on the main thread, and set before the handle is handed to
+ // the ScriptExecutorRunnable that releases the request, which is what makes
+ // it safe to read on the main thread without synchronization.
+ bool mExecutionScheduled = false;
+
+ // The request is released on the worker thread by ReleaseRequest(), which
+ // only runs once the handle has been handed off to a ScriptExecutorRunnable,
+ // and the worker may drop the last reference to it immediately afterwards.
+ // Main-thread code therefore cannot test mRequest to decide whether the
+ // request is still around: that read races with the worker thread and can
+ // hand back an already freed request. Gate on ExecutionScheduled() instead.
RefPtr<JS::loader::ScriptLoadRequest> mRequest;
nsCOMPtr<nsISerialEventTarget> mOwningEventTarget;
};
Loading diff…
References
On This Page