Skip to content

Commit

Permalink
[WebAudio] Use-after-free in WebCore::AudioBufferSourceNode::renderFr…
Browse files Browse the repository at this point in the history
…omBuffer

https://bugs.webkit.org/show_bug.cgi?id=272607
rdar://126326144

Reviewed by Yusuke Suzuki.

The JS on the main thread can detach the AudioBuffer's channels while
it is being read by the audio rendering thread, causing use-after-frees.

In a previous fix attempt, we starting copying the AudioBuffer's channels
so that the audio thread would read a copy instead. However, the increased
memory usage resulted in increased jetsams on gaming sites.

As a temporary stop gap measure, this patch simply marks the AudioBuffer's
channels as non-detachable to prevent the issue. This is not quite spec
compliant but it addresses the security issue until we can implement the
specification correctly without causing jetsams.

* Source/WebCore/Modules/webaudio/AudioBuffer.cpp:
(WebCore::AudioBuffer::markBuffersAsNonDetachable):
* Source/WebCore/Modules/webaudio/AudioBuffer.h:
* Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp:
(WebCore::AudioBufferSourceNode::acquireBufferContent):
(WebCore::AudioBufferSourceNode::setBufferForBindings):
(WebCore::AudioBufferSourceNode::startPlaying):
* Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h:

Canonical link: https://commits.webkit.org/272448.925@safari-7618-branch
  • Loading branch information
cdumez committed Apr 13, 2024
1 parent 19eab2f commit 4201e96
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 0 deletions.
13 changes: 13 additions & 0 deletions Source/WebCore/Modules/webaudio/AudioBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,19 @@ AudioBuffer::AudioBuffer(AudioBus& bus)
m_channelWrappers = FixedVector<JSValueInWrappedObject> { m_channels.size() };
}

// FIXME: We don't currently implement "acquire the content" [1] and
// AudioBuffer::getChannelData() correctly. As a result, the audio thread
// may be reading an array buffer that the JS can detach. To guard against
// this, we mark the array buffers as non-detachable as soon as their content
// has been acquired.
// [1] https://www.w3.org/TR/webaudio/#acquire-the-content
void AudioBuffer::markBuffersAsNonDetachable()
{
Locker locker { m_channelsLock };
for (auto& channel : m_channels)
channel->setDetachable(false);
}

void AudioBuffer::invalidate()
{
releaseMemory();
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/Modules/webaudio/AudioBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ class AudioBuffer : public RefCounted<AudioBuffer> {
size_t length() const { return hasDetachedChannelBuffer() ? 0 : m_originalLength; }
double duration() const { return length() / static_cast<double>(sampleRate()); }

void markBuffersAsNonDetachable();

// Channel data access
unsigned numberOfChannels() const { return m_channels.size(); }
ExceptionOr<JSC::JSValue> getChannelData(JSDOMGlobalObject&, unsigned channelIndex);
Expand Down
13 changes: 13 additions & 0 deletions Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,15 @@ bool AudioBufferSourceNode::renderFromBuffer(AudioBus* bus, unsigned destination
return true;
}

void AudioBufferSourceNode::acquireBufferContent()
{
ASSERT(isMainThread());

// FIXME: We should implement https://www.w3.org/TR/webaudio/#acquire-the-content.
if (m_buffer)
m_buffer->markBuffersAsNonDetachable();
}

ExceptionOr<void> AudioBufferSourceNode::setBufferForBindings(RefPtr<AudioBuffer>&& buffer)
{
ASSERT(isMainThread());
Expand Down Expand Up @@ -446,6 +455,9 @@ ExceptionOr<void> AudioBufferSourceNode::setBufferForBindings(RefPtr<AudioBuffer
if (m_isGrain)
adjustGrainParameters();

if (isPlayingOrScheduled())
acquireBufferContent();

return { };
}

Expand Down Expand Up @@ -513,6 +525,7 @@ ExceptionOr<void> AudioBufferSourceNode::startPlaying(double when, double grainO

adjustGrainParameters();

acquireBufferContent();
m_playbackState = SCHEDULED_STATE;

return { };
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ class AudioBufferSourceNode final : public AudioScheduledSourceNode {
private:
AudioBufferSourceNode(BaseAudioContext&);

void acquireBufferContent() WTF_REQUIRES_LOCK(m_processLock);

double tailTime() const final { return 0; }
double latencyTime() const final { return 0; }

Expand Down

0 comments on commit 4201e96

Please sign in to comment.