Soundtrigger service: fix cross deadlock with audio policy service

Do not hold Module mutex when calling into audio policy manager to
avoid cross deadlock with audio poicy service mutex: Audio policy manager
can call into sound trigger service with its mutex held in methods like
stopInput().

Regression introduced by fix for b/64340921 commit f759b8c4

Bug: 64340921
Bug: 67310830
Test: repro steps in b/67310830
Merged-In: Ie50b2e7c55fe9828a3fd8de6b31eb4a492791583

Change-Id: Ie50b2e7c55fe9828a3fd8de6b31eb4a492791583
(cherry picked from commit 98647879efd7fd85c57399037a2cf330726b0a09)
CVE-2017-0837

2 files changed, 57 insertions, 44 deletions

diff --git a/services/soundtrigger/SoundTriggerHwService.cpp b/services/soundtrigger/SoundTriggerHwService.cpp
index a45d5f6ab8..4e54b919e7 100644
--- a/services/soundtrigger/SoundTriggerHwService.cpp
+++ b/services/soundtrigger/SoundTriggerHwService.cpp
@@ -528,6 +528,8 @@ void SoundTriggerHwService::Module::detach() {
     if (!captureHotwordAllowed()) {
         return;
     }
+    Vector<audio_session_t> releasedSessions;
+
     {
         AutoMutex lock(mLock);
         for (size_t i = 0; i < mModels.size(); i++) {
@@ -537,9 +539,16 @@ void SoundTriggerHwService::Module::detach() {
                 mHwDevice->stop_recognition(mHwDevice, model->mHandle);
             }
             mHwDevice->unload_sound_model(mHwDevice, model->mHandle);
+            releasedSessions.add(model->mCaptureSession);
         }
         mModels.clear();
     }
+
+    for (size_t i = 0; i < releasedSessions.size(); i++) {
+        // do not call AudioSystem methods with mLock held
+        AudioSystem::releaseSoundTriggerSession(releasedSessions[i]);
+    }
+
     if (mClient != 0) {
         IInterface::asBinder(mClient)->unlinkToDeath(this);
     }
@@ -581,37 +590,43 @@ status_t SoundTriggerHwService::Module::loadSoundModel(const sp<IMemory>& modelM
         return BAD_VALUE;
     }
 
-    AutoMutex lock(mLock);
-
-    if (mModels.size() >= mDescriptor.properties.max_sound_models) {
-        if (mModels.size() == 0) {
-            return INVALID_OPERATION;
-        }
-        ALOGW("loadSoundModel() max number of models exceeded %d making room for a new one",
-              mDescriptor.properties.max_sound_models);
-        unloadSoundModel_l(mModels.valueAt(0)->mHandle);
-    }
-
-    status_t status = mHwDevice->load_sound_model(mHwDevice,
-                                                  sound_model,
-                                                  SoundTriggerHwService::soundModelCallback,
-                                                  this,
-                                                  handle);
-    if (status != NO_ERROR) {
-        return status;
-    }
     audio_session_t session;
     audio_io_handle_t ioHandle;
     audio_devices_t device;
-
-    status = AudioSystem::acquireSoundTriggerSession(&session, &ioHandle, &device);
+    // do not call AudioSystem methods with mLock held
+    status_t status = AudioSystem::acquireSoundTriggerSession(&session, &ioHandle, &device);
     if (status != NO_ERROR) {
         return status;
     }
 
-    sp<Model> model = new Model(*handle, session, ioHandle, device, sound_model->type);
-    mModels.replaceValueFor(*handle, model);
+    {
+        AutoMutex lock(mLock);
+
+        if (mModels.size() >= mDescriptor.properties.max_sound_models) {
+            ALOGW("loadSoundModel(): Not loading, max number of models (%d) would be exceeded",
+                  mDescriptor.properties.max_sound_models);
+            status = INVALID_OPERATION;
+            goto exit;
+        }
 
+        status_t status = mHwDevice->load_sound_model(mHwDevice,
+                                                      sound_model,
+                                                      SoundTriggerHwService::soundModelCallback,
+                                                      this,
+                                                      handle);
+        if (status != NO_ERROR) {
+            goto exit;
+        }
+
+        sp<Model> model = new Model(*handle, session, ioHandle, device, sound_model->type);
+        mModels.replaceValueFor(*handle, model);
+    }
+
+exit:
+    if (status != NO_ERROR) {
+        // do not call AudioSystem methods with mLock held
+        AudioSystem::releaseSoundTriggerSession(session);
+    }
     return status;
 }
 
@@ -621,25 +636,26 @@ status_t SoundTriggerHwService::Module::unloadSoundModel(sound_model_handle_t ha
     if (!captureHotwordAllowed()) {
         return PERMISSION_DENIED;
     }
+    status_t status;
+    audio_session_t session;
 
-    AutoMutex lock(mLock);
-    return unloadSoundModel_l(handle);
-}
-
-status_t SoundTriggerHwService::Module::unloadSoundModel_l(sound_model_handle_t handle)
-{
-    ssize_t index = mModels.indexOfKey(handle);
-    if (index < 0) {
-        return BAD_VALUE;
-    }
-    sp<Model> model = mModels.valueAt(index);
-    mModels.removeItem(handle);
-    if (model->mState == Model::STATE_ACTIVE) {
-        mHwDevice->stop_recognition(mHwDevice, model->mHandle);
-        model->mState = Model::STATE_IDLE;
-    }
-    AudioSystem::releaseSoundTriggerSession(model->mCaptureSession);
-    return mHwDevice->unload_sound_model(mHwDevice, handle);
+    {
+      AutoMutex lock(mLock);
+      ssize_t index = mModels.indexOfKey(handle);
+      if (index < 0) {
+          return BAD_VALUE;
+      }
+      sp<Model> model = mModels.valueAt(index);
+      mModels.removeItem(handle);
+      if (model->mState == Model::STATE_ACTIVE) {
+          mHwDevice->stop_recognition(mHwDevice, model->mHandle);
+          model->mState = Model::STATE_IDLE;
+      }
+      status = mHwDevice->unload_sound_model(mHwDevice, handle);
+      session = model->mCaptureSession;
+    }
+    AudioSystem::releaseSoundTriggerSession(session);
+    return status;
 }
 
 status_t SoundTriggerHwService::Module::startRecognition(sound_model_handle_t handle,
diff --git a/services/soundtrigger/SoundTriggerHwService.h b/services/soundtrigger/SoundTriggerHwService.h
index 2619a5fb49..d05dacdb7f 100644
--- a/services/soundtrigger/SoundTriggerHwService.h
+++ b/services/soundtrigger/SoundTriggerHwService.h
@@ -141,9 +141,6 @@ public:
 
     private:
 
-       status_t unloadSoundModel_l(sound_model_handle_t handle);
-
-
         Mutex                                  mLock;
         wp<SoundTriggerHwService>              mService;
         struct sound_trigger_hw_device*        mHwDevice;