commit 3e9afc163256db661b9039120d07501b3a8a7d99
author Arthur Ishiguro <arthuri@google.com> Tue Sep 22 13:05:15 2020 -0700
committer Arthur Ishiguro <arthuri@google.com> Fri Oct 02 11:24:49 2020 -0700
tree fa4ec4960f9d248093be44c5d49d75a7907c2d85
parent a72accd83cc7f7144ad19439c24d4db7a1572141

Prevent mEventCache UAF in SensorEventConnection

Since there is no check to see if SensorEventConnection has been
destroyed, the mEventCache pointer can still be used even after it
was freed.

Bug: 168211968
Test: Run test code that attempts to enable a sensor after destroying
the SensorEventConnection, and verify no system_server crash occurs.

Change-Id: Ia9275b7cc574df371cdb2e1b80c6699df193b580

services/sensorservice/SensorEventConnection.cpp

diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp
index d14a301..3cccaf9 100644
--- a/services/sensorservice/SensorEventConnection.cpp
+++ b/services/sensorservice/SensorEventConnection.cpp
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <log/log.h>
 #include <sys/socket.h>
 #include <utils/threads.h>
 
@@ -53,20 +54,13 @@
 SensorService::SensorEventConnection::~SensorEventConnection() {
     ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this);
     destroy();
-}
-
-void SensorService::SensorEventConnection::destroy() {
-    Mutex::Autolock _l(mDestroyLock);
-
-    // destroy once only
-    if (mDestroyed) {
-        return;
-    }
-
     mService->cleanupConnection(this);
     if (mEventCache != nullptr) {
         delete[] mEventCache;
     }
+}
+
+void SensorService::SensorEventConnection::destroy() {
     mDestroyed = true;
 }
 
@@ -679,6 +673,11 @@
         int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs,
         int reservedFlags)
 {
+    if (mDestroyed) {
+        android_errorWriteLog(0x534e4554, "168211968");
+        return DEAD_OBJECT;
+    }
+
     status_t err;
     if (enabled) {
         err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs,
@@ -693,10 +692,19 @@
 status_t SensorService::SensorEventConnection::setEventRate(
         int handle, nsecs_t samplingPeriodNs)
 {
+    if (mDestroyed) {
+        android_errorWriteLog(0x534e4554, "168211968");
+        return DEAD_OBJECT;
+    }
+
     return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName);
 }
 
 status_t  SensorService::SensorEventConnection::flush() {
+    if (mDestroyed) {
+        return DEAD_OBJECT;
+    }
+
     return  mService->flushSensor(this, mOpPackageName);
 }