Skip to content

Commit 7789ccf

Browse files
authored
media: fix heap-use-after-free in CobaltAudioRendererSink (youtube#9035)
Refactor CobaltAudioRendererSink to manage SbAudioSink lifecycle with std::unique_ptr and a custom deleter. This guarantees proper destruction of the audio sink when the renderer sink is destroyed or reset. This change prevents background ALSA threads from accessing a freed SbAudioSink object, addressing a heap-use-after-free ASAN error seen when UpdateSourceStatus was called on a freed instance. Issue: 483384414
1 parent b52b665 commit 7789ccf

File tree

2 files changed

+22
-10
lines changed

2 files changed

+22
-10
lines changed

cobalt/media/audio/cobalt_audio_renderer_sink.cc

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ int AlignUp(int value, int alignment) {
3131
}
3232
} // namespace
3333

34+
void SbAudioSinkDeleter::operator()(SbAudioSink sink) const {
35+
SbAudioSinkDestroy(sink);
36+
}
37+
3438
void CobaltAudioRendererSink::Initialize(const AudioParameters& params,
3539
RenderCallback* callback) {
3640
LOG(INFO) << "CobaltAudioRendererSink::Initialize - called with following "
@@ -78,17 +82,17 @@ void CobaltAudioRendererSink::Start() {
7882
}
7983

8084
output_frame_buffers_[0] = output_frame_buffer_.get();
81-
audio_sink_ = SbAudioSinkCreate(
85+
audio_sink_ = AudioSinkUniquePtr(SbAudioSinkCreate(
8286
params_.channels(), nearest_supported_sample_rate_, output_sample_type_,
8387
kSbMediaAudioFrameStorageTypeInterleaved, &output_frame_buffers_[0],
8488
frames_per_channel_, &CobaltAudioRendererSink::UpdateSourceStatusFunc,
85-
&CobaltAudioRendererSink::ConsumeFramesFunc, this);
86-
DCHECK(SbAudioSinkIsValid(audio_sink_));
89+
&CobaltAudioRendererSink::ConsumeFramesFunc, /*context=*/this));
90+
CHECK(audio_sink_);
8791
}
8892

8993
void CobaltAudioRendererSink::Stop() {
9094
is_eos_reached_.store(true);
91-
SbAudioSinkDestroy(audio_sink_);
95+
audio_sink_.reset();
9296
callback_ = nullptr;
9397
}
9498

@@ -101,22 +105,22 @@ void CobaltAudioRendererSink::Pause() {
101105
}
102106

103107
void CobaltAudioRendererSink::Flush() {
104-
if (!SbAudioSinkIsValid(audio_sink_)) {
108+
if (!audio_sink_) {
105109
return;
106110
}
107111
// TODO(b/390468794) consolidate this recreation as it duplicates code from
108112
// Stop() and Start()
109-
SbAudioSinkDestroy(audio_sink_);
113+
audio_sink_.reset();
110114
frames_rendered_ = 0;
111115
frames_consumed_ = 0;
112-
audio_sink_ = SbAudioSinkCreate(
116+
audio_sink_ = AudioSinkUniquePtr(SbAudioSinkCreate(
113117
params_.channels(),
114118
SbAudioSinkGetNearestSupportedSampleFrequency(params_.sample_rate()),
115119
output_sample_type_, kSbMediaAudioFrameStorageTypeInterleaved,
116120
&output_frame_buffers_[0], frames_per_channel_,
117121
&CobaltAudioRendererSink::UpdateSourceStatusFunc,
118-
&CobaltAudioRendererSink::ConsumeFramesFunc, this);
119-
DCHECK(SbAudioSinkIsValid(audio_sink_));
122+
&CobaltAudioRendererSink::ConsumeFramesFunc, /*context=*/this));
123+
CHECK(audio_sink_);
120124
}
121125

122126
bool CobaltAudioRendererSink::SetVolume(double volume) {

cobalt/media/audio/cobalt_audio_renderer_sink.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#define COBALT_MEDIA_AUDIO_COBALT_AUDIO_RENDERER_SINK_H_
1717

1818
#include <atomic>
19+
#include <memory>
1920

2021
#include "media/base/audio_renderer_sink.h"
2122
#include "media/base/media_export.h"
@@ -24,6 +25,12 @@
2425

2526
namespace media {
2627

28+
// Custom deleter for Starboard Audio Sink.
29+
struct SbAudioSinkDeleter {
30+
using pointer = SbAudioSink;
31+
void operator()(SbAudioSink sink) const;
32+
};
33+
2734
class MEDIA_EXPORT CobaltAudioRendererSink final : public AudioRendererSink {
2835
public:
2936
CobaltAudioRendererSink() = default;
@@ -84,7 +91,8 @@ class MEDIA_EXPORT CobaltAudioRendererSink final : public AudioRendererSink {
8491
std::atomic<bool> is_playing_ = false;
8592
std::atomic<bool> is_eos_reached_ = false;
8693

87-
SbAudioSink audio_sink_ = kSbAudioSinkInvalid;
94+
using AudioSinkUniquePtr = std::unique_ptr<SbAudioSink, SbAudioSinkDeleter>;
95+
AudioSinkUniquePtr audio_sink_;
8896

8997
double resample_ratio_ = 1.0;
9098
std::unique_ptr<MultiChannelResampler> resampler_ = nullptr;

0 commit comments

Comments
 (0)