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

Commit 930b5835 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
Merged-In: Ia9275b7cc574df371cdb2e1b80c6699df193b580
(cherry picked from commit 3e9afc16)
parent 66909092

services/sensorservice/SensorEventConnection.cpp

+18 −10
Original line number Original line Diff line number Diff line
@@ -14,6 +14,7 @@ 
 * limitations under the License.

 * limitations under the License.

 */

 */





#include <log/log.h>

#include <sys/socket.h>

#include <sys/socket.h>

#include <utils/threads.h>

#include <utils/threads.h>
@@ -53,20 +54,13 @@ SensorService::SensorEventConnection::SensorEventConnection( 
SensorService::SensorEventConnection::~SensorEventConnection() {

SensorService::SensorEventConnection::~SensorEventConnection() {

    ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this);

    ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this);

    destroy();

    destroy();

}



void SensorService::SensorEventConnection::destroy() {

    Mutex::Autolock _l(mDestroyLock);



    // destroy once only

    if (mDestroyed) {

        return;

    }



    mService->cleanupConnection(this);

    mService->cleanupConnection(this);

    if (mEventCache != nullptr) {

    if (mEventCache != nullptr) {

        delete[] mEventCache;

        delete[] mEventCache;

    }

    }

}



void SensorService::SensorEventConnection::destroy() {

    mDestroyed = true;

    mDestroyed = true;

}

}
@@ -679,6 +673,11 @@ status_t SensorService::SensorEventConnection::enableDisable( 
        int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs,

        int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs,

        int reservedFlags)

        int reservedFlags)

{

{

    if (mDestroyed) {

        android_errorWriteLog(0x534e4554, "168211968");

        return DEAD_OBJECT;

    }



    status_t err;

    status_t err;

    if (enabled) {

    if (enabled) {

        err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs,

        err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs,
@@ -693,10 +692,19 @@ status_t SensorService::SensorEventConnection::enableDisable( 
status_t SensorService::SensorEventConnection::setEventRate(

status_t SensorService::SensorEventConnection::setEventRate(

        int handle, nsecs_t samplingPeriodNs)

        int handle, nsecs_t samplingPeriodNs)

{

{

    if (mDestroyed) {

        android_errorWriteLog(0x534e4554, "168211968");

        return DEAD_OBJECT;

    }



    return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName);

    return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName);

}

}





status_t  SensorService::SensorEventConnection::flush() {

status_t  SensorService::SensorEventConnection::flush() {

    if (mDestroyed) {

        return DEAD_OBJECT;

    }



    return  mService->flushSensor(this, mOpPackageName);

    return  mService->flushSensor(this, mOpPackageName);

}

}

services/sensorservice/SensorEventConnection.h

+3 −2
Original line number Original line Diff line number Diff line
@@ -17,6 +17,7 @@ 
#ifndef ANDROID_SENSOR_EVENT_CONNECTION_H

#ifndef ANDROID_SENSOR_EVENT_CONNECTION_H

#define ANDROID_SENSOR_EVENT_CONNECTION_H

#define ANDROID_SENSOR_EVENT_CONNECTION_H





#include <atomic>

#include <stdint.h>

#include <stdint.h>

#include <sys/types.h>

#include <sys/types.h>

#include <unordered_map>

#include <unordered_map>
@@ -182,8 +183,8 @@ private: 
    int mTotalAcksNeeded, mTotalAcksReceived;

    int mTotalAcksNeeded, mTotalAcksReceived;

#endif

#endif





    mutable Mutex mDestroyLock;

    // Used to track if this object was inappropriately used after destroy().

    bool mDestroyed;

    std::atomic_bool mDestroyed;





    // Store a mapping of sensor handles to required AppOp for a sensor. This map only contains a

    // 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.

    // valid mapping for sensors that require a permission in order to reduce the lookup time.