Fix a possible stack corruption in AssistantAudioDecoder.

In OpenDecoderOnMediaThread(), the error callback to
media::BlockingUrlProtocol is being bound to a stack variable. However,
the BlockingUrlProtocol (and callback) outlives the function. So any
read error that occurs later, such as in
AssistantAudioDecoder::Decode(), will cause a write to the out-of-scope
variable, causing stack corruption.

Bug: None
Change-Id: I1149e48416c1a6ca2d8ddd9d9f124cf3bb6d5f8f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2542930Reviewed-by: Jeroen Dhollander <jeroendh@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: Anand K Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828083}

chromeos/services/assistant/audio_decoder/assistant_audio_decoder.cc

@@ -23,10 +23,6 @@ namespace {
 // Preferred bytes per sample when get interleaved data from AudioBus.
 constexpr int kBytesPerSample = 2;
 
-void OnError(bool* succeeded) {
-  *succeeded = false;
-}
-
 }  // namespace
 
 AssistantAudioDecoder::AssistantAudioDecoder(
@@ -68,13 +64,18 @@ void AssistantAudioDecoder::CloseDecoder(CloseDecoderCallback callback) {
                      base::Unretained(this)));
 }
 
+void AssistantAudioDecoder::OnDataReadError() {
+  read_error_ = true;
+}
+
 void AssistantAudioDecoder::OpenDecoderOnMediaThread() {
-  bool read_ok = true;
   protocol_ = std::make_unique<media::BlockingUrlProtocol>(
-      data_source_.get(), base::BindRepeating(&OnError, &read_ok));
+      data_source_.get(),
+      base::BindRepeating(&AssistantAudioDecoder::OnDataReadError,
+                          base::Unretained(this)));
   decoder_ = std::make_unique<media::AudioFileReader>(protocol_.get());
 
-  if (closed_ || !decoder_->Open() || !read_ok) {
+  if (closed_ || !decoder_->Open() || read_error_) {
     CloseDecoderOnMediaThread();
     return;
   }

chromeos/services/assistant/audio_decoder/assistant_audio_decoder.h

@@ -46,6 +46,9 @@ class AssistantAudioDecoder : public mojom::AssistantAudioDecoder {
       const std::vector<std::unique_ptr<media::AudioBus>>&
           decoded_audio_buffers);
 
+  // Error callback for media::BlockingUrlProtocol. Only run on media thread.
+  void OnDataReadError();
+
   void OnConnectionError();
   void RunCallbacksAsClosed();
 
@@ -55,6 +58,7 @@ class AssistantAudioDecoder : public mojom::AssistantAudioDecoder {
   OpenDecoderCallback open_callback_;
   CloseDecoderCallback close_callback_;
   bool closed_ = false;
+  bool read_error_ = false;
 
   std::unique_ptr<media::DataSource> data_source_;
   std::unique_ptr<media::BlockingUrlProtocol> protocol_;