From 139f7c23898bff49ea6eb9a1f6cdb7e43d28d8ac Mon Sep 17 00:00:00 2001
From: Ytai Ben-Tsvi <ytai@google.com>
Date: Tue, 19 Jan 2021 11:55:09 -0800
Subject: [PATCH] Fix race condition in SoundTriggerHw2Enforcer

Since HIDL doesn't guarantee message ordering between sync calls into
the HAL and async calls coming from the HAL, we have a rare
possibility of a race condition if a detection event callback arrives
before the startRecognition call returns. This change addresses that.
We do so by holding the lock during the calls into the HAL. We do NOT
hold the lock during the the "upward" calls to avoid deadlocks.

Fixes: 177795410
Test: Manual verification of STHAL operation.
      Sent patch to bug reporter to apply and test.
Change-Id: Ie042a92afa998d6d78786942146e3c533c401cd5
---
 .../SoundTriggerHw2Enforcer.java              | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Enforcer.java b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Enforcer.java
index eced8940947ac..90ac69a971582 100644
--- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Enforcer.java
+++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Enforcer.java
@@ -42,7 +42,7 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     static final String TAG = "SoundTriggerHw2Enforcer";
 
     final ISoundTriggerHw2 mUnderlying;
-    Map<Integer, Boolean> mModelStates = new HashMap<>();
+    final Map<Integer, Boolean> mModelStates = new HashMap<>();
 
     public SoundTriggerHw2Enforcer(
             ISoundTriggerHw2 underlying) {
@@ -62,12 +62,12 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     public int loadSoundModel(ISoundTriggerHw.SoundModel soundModel, Callback callback,
             int cookie) {
         try {
-            int handle = mUnderlying.loadSoundModel(soundModel, new CallbackEnforcer(callback),
-                    cookie);
             synchronized (mModelStates) {
+                int handle = mUnderlying.loadSoundModel(soundModel, new CallbackEnforcer(callback),
+                        cookie);
                 mModelStates.put(handle, false);
+                return handle;
             }
-            return handle;
         } catch (RuntimeException e) {
             throw handleException(e);
         }
@@ -77,13 +77,13 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     public int loadPhraseSoundModel(ISoundTriggerHw.PhraseSoundModel soundModel, Callback callback,
             int cookie) {
         try {
-            int handle = mUnderlying.loadPhraseSoundModel(soundModel,
-                    new CallbackEnforcer(callback),
-                    cookie);
             synchronized (mModelStates) {
+                int handle = mUnderlying.loadPhraseSoundModel(soundModel,
+                        new CallbackEnforcer(callback),
+                        cookie);
                 mModelStates.put(handle, false);
+                return handle;
             }
-            return handle;
         } catch (RuntimeException e) {
             throw handleException(e);
         }
@@ -92,8 +92,8 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     @Override
     public void unloadSoundModel(int modelHandle) {
         try {
-            mUnderlying.unloadSoundModel(modelHandle);
             synchronized (mModelStates) {
+                mUnderlying.unloadSoundModel(modelHandle);
                 mModelStates.remove(modelHandle);
             }
         } catch (RuntimeException e) {
@@ -104,8 +104,8 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     @Override
     public void stopRecognition(int modelHandle) {
         try {
-            mUnderlying.stopRecognition(modelHandle);
             synchronized (mModelStates) {
+                mUnderlying.stopRecognition(modelHandle);
                 mModelStates.replace(modelHandle, false);
             }
         } catch (RuntimeException e) {
@@ -116,8 +116,8 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     @Override
     public void stopAllRecognitions() {
         try {
-            mUnderlying.stopAllRecognitions();
             synchronized (mModelStates) {
+                mUnderlying.stopAllRecognitions();
                 for (Map.Entry<Integer, Boolean> entry : mModelStates.entrySet()) {
                     entry.setValue(false);
                 }
@@ -131,9 +131,9 @@ public class SoundTriggerHw2Enforcer implements ISoundTriggerHw2 {
     public void startRecognition(int modelHandle, RecognitionConfig config, Callback callback,
             int cookie) {
         try {
-            mUnderlying.startRecognition(modelHandle, config, new CallbackEnforcer(callback),
-                    cookie);
             synchronized (mModelStates) {
+                mUnderlying.startRecognition(modelHandle, config, new CallbackEnforcer(callback),
+                        cookie);
                 mModelStates.replace(modelHandle, true);
             }
         } catch (RuntimeException e) {