From c92e99aa5f826d890cc22bc709039986a27968db Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Thu, 6 Sep 2018 11:37:22 +0200 Subject: [Backport] CVE-2018-16067 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audio thread should not access destination node The AudioDestinationNode is an object managed by Oilpan so the audio thread should not access it. However, the audio thread needs information (currentTime, etc) from the destination node. So instead of accessing the audio destination handler (a scoped_refptr) via the destination node, add a new member to the base audio context that holds onto the destination handler. The destination handler is not an oilpan object and lives at least as long as the base audio context. Bug: 860626, 860522, 863951 Change-Id: I5d4d5e82c09bea552f0866b52515878683b87f3a Test: Test case from 860522 doesn't crash on asan build Reviewed-on: https://chromium-review.googlesource.com/1138974 Reviewed-by: Michael Brüning --- .../Source/modules/webaudio/BaseAudioContext.cpp | 8 ++++++++ .../Source/modules/webaudio/BaseAudioContext.h | 23 ++++++---------------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/chromium/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp b/chromium/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp index f391b7f7694..4bf87496d4d 100644 --- a/chromium/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp +++ b/chromium/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp @@ -153,6 +153,14 @@ void BaseAudioContext::Initialize() { if (destination_node_) { destination_node_->Handler().Initialize(); + // TODO(crbug.com/863951). The audio thread needs some things from the + // destination handler like the currentTime. But the audio thread + // shouldn't access the |destination_node_| since it's an Oilpan object. + // Thus, get the destination handler, a non-oilpan object, so we can get + // the items directly from the handler instead of through the destination + // node. + destination_handler_ = &destination_node_->GetAudioDestinationHandler(); + // The AudioParams in the listener need access to the destination node, so // only create the listener if the destination node exists. listener_ = AudioListener::Create(*this); diff --git a/chromium/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.h b/chromium/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.h index f303c89d692..050fd323ab1 100644 --- a/chromium/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.h +++ b/chromium/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.h @@ -128,26 +128,12 @@ class MODULES_EXPORT BaseAudioContext AudioDestinationNode* destination() const; size_t CurrentSampleFrame() const { - // TODO: What is the correct value for the current frame if the destination - // node has gone away? 0 is a valid frame. - return destination_node_ ? destination_node_->GetAudioDestinationHandler() - .CurrentSampleFrame() - : 0; + return destination_handler_->CurrentSampleFrame(); } - double currentTime() const { - // TODO: What is the correct value for the current time if the destination - // node has gone away? 0 is a valid time. - return destination_node_ - ? destination_node_->GetAudioDestinationHandler().CurrentTime() - : 0; - } + double currentTime() const { return destination_handler_->CurrentTime(); } - float sampleRate() const { - return destination_node_ - ? destination_node_->GetAudioDestinationHandler().SampleRate() - : ClosedContextSampleRate(); - } + float sampleRate() const { return destination_handler_->SampleRate(); } float FramesPerBuffer() const { return destination_node_ ? destination_node_->GetAudioDestinationHandler() @@ -538,6 +524,9 @@ class MODULES_EXPORT BaseAudioContext Optional autoplay_status_; AudioIOPosition output_position_; + // The handler associated with the above |destination_node_|. + scoped_refptr destination_handler_; + Member audio_worklet_; }; -- cgit v1.2.3