From a97e74afa987382c0f6b10eefc14cbf49e079c85 Mon Sep 17 00:00:00 2001 From: Hongchan Choi Date: Wed, 4 Aug 2021 01:25:36 +0000 Subject: [Backport] CVE-2021-30603: Race in WebAudio Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/3068260: Protect HRTF database loader thread from access by different threads This patch add a new mutex locker around the HRTF database loader thread to ensure the safe exclusive access of the loader thread and the HRTF database. Bug: 1233564 Change-Id: Ie12b99ffe520d3747e34af387a37637a10aab38a Auto-Submit: Hongchan Choi Commit-Queue: Kentaro Hara Reviewed-by: Kentaro Hara Cr-Commit-Position: refs/heads/master@{#908269} Reviewed-by: Allan Sandfeld Jensen --- .../blink/renderer/platform/audio/hrtf_database_loader.cc | 6 ++++++ .../blink/renderer/platform/audio/hrtf_database_loader.h | 9 ++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/chromium/third_party/blink/renderer/platform/audio/hrtf_database_loader.cc b/chromium/third_party/blink/renderer/platform/audio/hrtf_database_loader.cc index ef2ca352a32..63386a98aa8 100644 --- a/chromium/third_party/blink/renderer/platform/audio/hrtf_database_loader.cc +++ b/chromium/third_party/blink/renderer/platform/audio/hrtf_database_loader.cc @@ -86,6 +86,8 @@ void HRTFDatabaseLoader::LoadTask() { void HRTFDatabaseLoader::LoadAsynchronously() { DCHECK(IsMainThread()); + MutexLocker locker(lock_); + // m_hrtfDatabase and m_thread should both be unset because this should be a // new HRTFDatabaseLoader object that was just created by // createAndLoadAsynchronouslyIfNecessary and because we haven't started @@ -122,6 +124,10 @@ void HRTFDatabaseLoader::CleanupTask(WaitableEvent* sync) { } void HRTFDatabaseLoader::WaitForLoaderThreadCompletion() { + // We can lock this because this is called from either the main thread or + // the offline audio rendering thread. + MutexLocker locker(lock_); + if (!thread_) return; diff --git a/chromium/third_party/blink/renderer/platform/audio/hrtf_database_loader.h b/chromium/third_party/blink/renderer/platform/audio/hrtf_database_loader.h index a17ffb98dce..307272ac9a2 100644 --- a/chromium/third_party/blink/renderer/platform/audio/hrtf_database_loader.h +++ b/chromium/third_party/blink/renderer/platform/audio/hrtf_database_loader.h @@ -60,8 +60,8 @@ class PLATFORM_EXPORT HRTFDatabaseLoader final // must be called from the audio thread. bool IsLoaded() { return Database(); } - // waitForLoaderThreadCompletion() may be called more than once and is - // thread-safe. + // May be called from both main and audio thread, and also can be called more + // than once. void WaitForLoaderThreadCompletion(); // Returns the database or nullptr if the database doesn't yet exist. Must @@ -83,11 +83,10 @@ class PLATFORM_EXPORT HRTFDatabaseLoader final void LoadTask(); void CleanupTask(WaitableEvent*); - // Holding a m_lock is required when accessing m_hrtfDatabase since we access - // it from multiple threads. + // |lock_| MUST be held when accessing |hrtf_database_| or |thread_| because + // it can be accessed by multiple threads (e.g multiple AudioContexts). Mutex lock_; std::unique_ptr hrtf_database_; - std::unique_ptr thread_; float database_sample_rate_; -- cgit v1.2.3