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 Merged-In: Ia9275b7cc574df371cdb2e1b80c6699df193b580 (cherry picked from commit 3e9afc163256db661b9039120d07501b3a8a7d99) (cherry picked from commit 930b583557e46f14b6d0a0dc1700eb33f18a7714)
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); }
diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h index 8f2d5db..9487a39 100644 --- a/services/sensorservice/SensorEventConnection.h +++ b/services/sensorservice/SensorEventConnection.h
@@ -17,6 +17,7 @@ #ifndef ANDROID_SENSOR_EVENT_CONNECTION_H #define ANDROID_SENSOR_EVENT_CONNECTION_H +#include <atomic> #include <stdint.h> #include <sys/types.h> #include <unordered_map> @@ -182,8 +183,8 @@ int mTotalAcksNeeded, mTotalAcksReceived; #endif - mutable Mutex mDestroyLock; - bool mDestroyed; + // Used to track if this object was inappropriately used after destroy(). + std::atomic_bool mDestroyed; // Store a mapping of sensor handles to required AppOp for a sensor. This map only contains a // valid mapping for sensors that require a permission in order to reduce the lookup time.