Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3e9afc16 authored by Arthur Ishiguro's avatar Arthur Ishiguro
Browse files

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
parent a72accd8

services/sensorservice/SensorEventConnection.cpp

+18 −10
Original line number Diff line number Diff line
@@ -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( 
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 @@ status_t SensorService::SensorEventConnection::enableDisable( 
        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::enableDisable( 
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);

}

services/sensorservice/SensorEventConnection.h

+3 −2
Original line number Diff line number Diff line
@@ -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 @@ private: 
    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.