audiohal: Fix UAF of HAL devices in Stream objects

Stream objects used to hold a pointer to underlying HAL device
object which they didn't own. Since destruction of server side
objects is asynchronous, it was possible that a Device object
gets destroyed before Stream objects, making all the HAL device
object pointer to become stale.

Fixed by adding a strong reference to Device objects into Stream
objects.

Bug: 36702804
Change-Id: I3da3611afbb91d6fd6410ac5b8af2a2eebfa6dac
Test: ran Loopback app and HAL VTS tests
(cherry picked from commit 96d3573cda)

audio/2.0/default/Device.cpp

@@ -59,6 +59,14 @@ Result Device::analyzeStatus(const char* funcName, int status) {
     }
 }
 
+void Device::closeInputStream(audio_stream_in_t* stream) {
+    mDevice->close_input_stream(mDevice, stream);
+}
+
+void Device::closeOutputStream(audio_stream_out_t* stream) {
+    mDevice->close_output_stream(mDevice, stream);
+}
+
 char* Device::halGetParameters(const char* keys) {
     return mDevice->get_parameters(mDevice, keys);
 }
@@ -160,7 +168,7 @@ Return<void> Device::openOutputStream(
     ALOGV("open_output_stream status %d stream %p", status, halStream);
     sp<IStreamOut> streamOut;
     if (status == OK) {
-        streamOut = new StreamOut(mDevice, halStream);
+        streamOut = new StreamOut(this, halStream);
     }
     AudioConfig suggestedConfig;
     HidlUtils::audioConfigFromHal(halConfig, &suggestedConfig);
@@ -196,7 +204,7 @@ Return<void> Device::openInputStream(
     ALOGV("open_input_stream status %d stream %p", status, halStream);
     sp<IStreamIn> streamIn;
     if (status == OK) {
-        streamIn = new StreamIn(mDevice, halStream);
+        streamIn = new StreamIn(this, halStream);
     }
     AudioConfig suggestedConfig;
     HidlUtils::audioConfigFromHal(halConfig, &suggestedConfig);

audio/2.0/default/Device.h

@@ -98,6 +98,8 @@ struct Device : public IDevice, public ParametersUtil {
 
     // Utility methods for extending interfaces.
     Result analyzeStatus(const char* funcName, int status);
+    void closeInputStream(audio_stream_in_t* stream);
+    void closeOutputStream(audio_stream_out_t* stream);
     audio_hw_device_t* device() const { return mDevice; }
 
   private:

audio/2.0/default/StreamIn.cpp

@@ -135,7 +135,7 @@ bool ReadThread::threadLoop() {
 
 }  // namespace
 
-StreamIn::StreamIn(audio_hw_device_t* device, audio_stream_in_t* stream)
+StreamIn::StreamIn(const sp<Device>& device, audio_stream_in_t* stream)
         : mIsClosed(false), mDevice(device), mStream(stream),
           mStreamCommon(new Stream(&stream->common)),
           mStreamMmap(new StreamMmap<audio_stream_in_t>(stream)),
@@ -154,9 +154,8 @@ StreamIn::~StreamIn() {
         status_t status = EventFlag::deleteEventFlag(&mEfGroup);
         ALOGE_IF(status, "read MQ event flag deletion error: %s", strerror(-status));
     }
-    mDevice->close_input_stream(mDevice, mStream);
+    mDevice->closeInputStream(mStream);
     mStream = nullptr;
-    mDevice = nullptr;
 }
 
 // Methods from ::android::hardware::audio::V2_0::IStream follow.

audio/2.0/default/StreamIn.h

@@ -27,6 +27,7 @@
 #include <hidl/Status.h>
 #include <utils/Thread.h>
 
+#include "Device.h"
 #include "Stream.h"
 
 namespace android {
@@ -55,7 +56,7 @@ struct StreamIn : public IStreamIn {
     typedef MessageQueue<uint8_t, kSynchronizedReadWrite> DataMQ;
     typedef MessageQueue<ReadStatus, kSynchronizedReadWrite> StatusMQ;
 
-    StreamIn(audio_hw_device_t* device, audio_stream_in_t* stream);
+    StreamIn(const sp<Device>& device, audio_stream_in_t* stream);
 
     // Methods from ::android::hardware::audio::V2_0::IStream follow.
     Return<uint64_t> getFrameSize()  override;
@@ -101,10 +102,10 @@ struct StreamIn : public IStreamIn {
 
   private:
     bool mIsClosed;
-    audio_hw_device_t *mDevice;
+    const sp<Device> mDevice;
     audio_stream_in_t *mStream;
-    sp<Stream> mStreamCommon;
-    sp<StreamMmap<audio_stream_in_t>> mStreamMmap;
+    const sp<Stream> mStreamCommon;
+    const sp<StreamMmap<audio_stream_in_t>> mStreamMmap;
     std::unique_ptr<CommandMQ> mCommandMQ;
     std::unique_ptr<DataMQ> mDataMQ;
     std::unique_ptr<StatusMQ> mStatusMQ;

audio/2.0/default/StreamOut.cpp

@@ -135,7 +135,7 @@ bool WriteThread::threadLoop() {
 
 }  // namespace
 
-StreamOut::StreamOut(audio_hw_device_t* device, audio_stream_out_t* stream)
+StreamOut::StreamOut(const sp<Device>& device, audio_stream_out_t* stream)
         : mIsClosed(false), mDevice(device), mStream(stream),
           mStreamCommon(new Stream(&stream->common)),
           mStreamMmap(new StreamMmap<audio_stream_out_t>(stream)),
@@ -155,9 +155,8 @@ StreamOut::~StreamOut() {
         ALOGE_IF(status, "write MQ event flag deletion error: %s", strerror(-status));
     }
     mCallback.clear();
-    mDevice->close_output_stream(mDevice, mStream);
+    mDevice->closeOutputStream(mStream);
     mStream = nullptr;
-    mDevice = nullptr;
 }
 
 // Methods from ::android::hardware::audio::V2_0::IStream follow.

audio/2.0/default/StreamOut.h

@@ -27,6 +27,7 @@
 #include <fmq/MessageQueue.h>
 #include <utils/Thread.h>
 
+#include "Device.h"
 #include "Stream.h"
 
 namespace android {
@@ -57,7 +58,7 @@ struct StreamOut : public IStreamOut {
     typedef MessageQueue<uint8_t, kSynchronizedReadWrite> DataMQ;
     typedef MessageQueue<WriteStatus, kSynchronizedReadWrite> StatusMQ;
 
-    StreamOut(audio_hw_device_t* device, audio_stream_out_t* stream);
+    StreamOut(const sp<Device>& device, audio_stream_out_t* stream);
 
     // Methods from ::android::hardware::audio::V2_0::IStream follow.
     Return<uint64_t> getFrameSize()  override;
@@ -112,10 +113,10 @@ struct StreamOut : public IStreamOut {
 
   private:
     bool mIsClosed;
-    audio_hw_device_t *mDevice;
+    const sp<Device> mDevice;
     audio_stream_out_t *mStream;
-    sp<Stream> mStreamCommon;
-    sp<StreamMmap<audio_stream_out_t>> mStreamMmap;
+    const sp<Stream> mStreamCommon;
+    const sp<StreamMmap<audio_stream_out_t>> mStreamMmap;
     sp<IStreamOutCallback> mCallback;
     std::unique_ptr<CommandMQ> mCommandMQ;
     std::unique_ptr<DataMQ> mDataMQ;