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

Commit e0721534 authored by Stan Rokita's avatar Stan Rokita
Browse files

MH2 | Check that subhal index is in range 

For each HalProxy method that takes in a sensorHandle, the code now
checks that the subhal index byte (first byte) of the handle is the
range of subhals in list before indexing into mSubHalList so that are
crash does not occur and returns Result::BAD_VALUE if that is the case.
Add unit tests to assert that error is returned. Also change the methods
that accept sensorHandles to int32_t type since that is the actual type
defined in the HAL spec.

Bug: 136511617
Test: Passing new unit tests.

Change-Id: I5489e999bc5eaef1a21698bdbc0a0341bc88194c
parent c913c85b

sensors/2.0/multihal/HalProxy.cpp

+36 −7
Original line number Diff line number Diff line
@@ -42,6 +42,8 @@ using ::android::hardware::sensors::V2_0::implementation::kWakelockTimeoutNs; 


typedef ISensorsSubHal*(SensorsHalGetSubHalFunc)(uint32_t*);



static constexpr int32_t kBitsAfterSubHalIndex = 24;



/**

 * Set the subhal index as first byte of sensor handle and return this modified version.

 *
@@ -50,8 +52,19 @@ typedef ISensorsSubHal*(SensorsHalGetSubHalFunc)(uint32_t*); 
 *

 * @return The modified sensor handle.

 */

uint32_t setSubHalIndex(uint32_t sensorHandle, size_t subHalIndex) {

    return sensorHandle | (subHalIndex << 24);

int32_t setSubHalIndex(int32_t sensorHandle, size_t subHalIndex) {

    return sensorHandle | (static_cast<int32_t>(subHalIndex) << kBitsAfterSubHalIndex);

}



/**

 * Extract the subHalIndex from sensorHandle.

 *

 * @param sensorHandle The sensorHandle to extract from.

 *

 * @return The subhal index.

 */

size_t extractSubHalIndex(int32_t sensorHandle) {

    return static_cast<size_t>(sensorHandle >> kBitsAfterSubHalIndex);

}



HalProxy::HalProxy() {
@@ -101,6 +114,9 @@ Return<Result> HalProxy::setOperationMode(OperationMode mode) { 
}



Return<Result> HalProxy::activate(int32_t sensorHandle, bool enabled) {

    if (!isSubHalIndexValid(sensorHandle)) {

        return Result::BAD_VALUE;

    }

    return getSubHalForSensorHandle(sensorHandle)

            ->activate(clearSubHalIndex(sensorHandle), enabled);

}
@@ -174,11 +190,17 @@ Return<Result> HalProxy::initialize( 


Return<Result> HalProxy::batch(int32_t sensorHandle, int64_t samplingPeriodNs,

                               int64_t maxReportLatencyNs) {

    if (!isSubHalIndexValid(sensorHandle)) {

        return Result::BAD_VALUE;

    }

    return getSubHalForSensorHandle(sensorHandle)

            ->batch(clearSubHalIndex(sensorHandle), samplingPeriodNs, maxReportLatencyNs);

}



Return<Result> HalProxy::flush(int32_t sensorHandle) {

    if (!isSubHalIndexValid(sensorHandle)) {

        return Result::BAD_VALUE;

    }

    return getSubHalForSensorHandle(sensorHandle)->flush(clearSubHalIndex(sensorHandle));

}
@@ -192,6 +214,9 @@ Return<Result> HalProxy::injectSensorData(const Event& event) { 
    }

    if (result == Result::OK) {

        Event subHalEvent = event;

        if (!isSubHalIndexValid(event.sensorHandle)) {

            return Result::BAD_VALUE;

        }

        subHalEvent.sensorHandle = clearSubHalIndex(event.sensorHandle);

        result = getSubHalForSensorHandle(event.sensorHandle)->injectSensorData(subHalEvent);

    }
@@ -326,7 +351,7 @@ void HalProxy::initializeSensorList() { 
                    ALOGE("SubHal sensorHandle's first byte was not 0");

                } else {

                    ALOGV("Loaded sensor: %s", sensor.name.c_str());

                    sensor.sensorHandle |= (subHalIndex << 24);

                    sensor.sensorHandle = setSubHalIndex(sensor.sensorHandle, subHalIndex);

                    setDirectChannelFlags(&sensor, subHal);

                    mSensors[sensor.sensorHandle] = sensor;

                }
@@ -539,8 +564,12 @@ void HalProxy::setDirectChannelFlags(SensorInfo* sensorInfo, ISensorsSubHal* sub 
    }

}



ISensorsSubHal* HalProxy::getSubHalForSensorHandle(uint32_t sensorHandle) {

    return mSubHalList[static_cast<size_t>(sensorHandle >> 24)];

ISensorsSubHal* HalProxy::getSubHalForSensorHandle(int32_t sensorHandle) {

    return mSubHalList[extractSubHalIndex(sensorHandle)];

}



bool HalProxy::isSubHalIndexValid(int32_t sensorHandle) {

    return extractSubHalIndex(sensorHandle) < mSubHalList.size();

}



size_t HalProxy::countNumWakeupEvents(const std::vector<Event>& events, size_t n) {
@@ -554,11 +583,11 @@ size_t HalProxy::countNumWakeupEvents(const std::vector<Event>& events, size_t n 
    return numWakeupEvents;

}



uint32_t HalProxy::clearSubHalIndex(uint32_t sensorHandle) {

int32_t HalProxy::clearSubHalIndex(int32_t sensorHandle) {

    return sensorHandle & (~kSensorHandleSubHalIndexMask);

}



bool HalProxy::subHalIndexIsClear(uint32_t sensorHandle) {

bool HalProxy::subHalIndexIsClear(int32_t sensorHandle) {

    return (sensorHandle & kSensorHandleSubHalIndexMask) == 0;

}

sensors/2.0/multihal/include/HalProxy.h

+16 −7
Original line number Diff line number Diff line
@@ -124,7 +124,7 @@ class HalProxy : public ISensors, public IScopedWakelockRefCounter { 
     *

     * @return The sensor info object in the mapping.

     */

    const SensorInfo& getSensorInfo(uint32_t sensorHandle) { return mSensors[sensorHandle]; }

    const SensorInfo& getSensorInfo(int32_t sensorHandle) { return mSensors[sensorHandle]; }



    bool areThreadsRunning() { return mThreadsRun.load(); }
@@ -177,10 +177,10 @@ class HalProxy : public ISensors, public IScopedWakelockRefCounter { 
     * The subhal index is encoded in the first byte of the sensor handle and the remaining

     * bytes are generated by the subhal to identify the sensor.

     */

    std::map<uint32_t, SensorInfo> mSensors;

    std::map<int32_t, SensorInfo> mSensors;



    //! Map of the dynamic sensors that have been added to halproxy.

    std::map<uint32_t, SensorInfo> mDynamicSensors;

    std::map<int32_t, SensorInfo> mDynamicSensors;



    //! The current operation mode for all subhals.

    OperationMode mCurrentOperationMode = OperationMode::NORMAL;
@@ -192,7 +192,7 @@ class HalProxy : public ISensors, public IScopedWakelockRefCounter { 
    static const int64_t kPendingWriteTimeoutNs = 5 * INT64_C(1000000000) /* 5 seconds */;



    //! The bit mask used to get the subhal index from a sensor handle.

    static constexpr uint32_t kSensorHandleSubHalIndexMask = 0xFF000000;

    static constexpr int32_t kSensorHandleSubHalIndexMask = 0xFF000000;



    /**

     * A FIFO queue of pairs of vector of events and the number of wakeup events in that vector
@@ -319,7 +319,16 @@ class HalProxy : public ISensors, public IScopedWakelockRefCounter { 
     *

     * @param sensorHandle The handle used to identify a sensor in one of the subhals.

     */

    ISensorsSubHal* getSubHalForSensorHandle(uint32_t sensorHandle);

    ISensorsSubHal* getSubHalForSensorHandle(int32_t sensorHandle);



    /**

     * Checks that sensorHandle's subhal index byte is within bounds of mSubHalList.

     *

     * @param sensorHandle The sensor handle to check.

     *

     * @return true if sensorHandles's subhal index byte is valid.

     */

    bool isSubHalIndexValid(int32_t sensorHandle);



    /**

     * Count the number of wakeup events in the first n events of the vector.
@@ -338,14 +347,14 @@ class HalProxy : public ISensors, public IScopedWakelockRefCounter { 
     *

     * @return The modified version of the sensor handle.

     */

    static uint32_t clearSubHalIndex(uint32_t sensorHandle);

    static int32_t clearSubHalIndex(int32_t sensorHandle);



    /**

     * @param sensorHandle The sensor handle to modify.

     *

     * @return true if subHalIndex byte of sensorHandle is zeroed.

     */

    static bool subHalIndexIsClear(uint32_t sensorHandle);

    static bool subHalIndexIsClear(int32_t sensorHandle);

};



/**

sensors/2.0/multihal/tests/HalProxy_test.cpp

+29 −0
Original line number Diff line number Diff line
@@ -663,6 +663,35 @@ TEST(HalProxyTest, DynamicSensorsDisconnectedTest) { 
    }

}



TEST(HalProxyTest, InvalidSensorHandleSubHalIndexProxyCalls) {

    constexpr size_t kNumSubHals = 3;

    constexpr size_t kQueueSize = 5;

    int32_t kNumSubHalsInt32 = static_cast<int32_t>(kNumSubHals);

    std::vector<AllSensorsSubHal> subHalObjs(kNumSubHals);

    std::vector<ISensorsSubHal*> subHals;

    for (const auto& subHal : subHalObjs) {

        subHals.push_back((ISensorsSubHal*)(&subHal));

    }



    std::unique_ptr<EventMessageQueue> eventQueue = makeEventFMQ(kQueueSize);

    std::unique_ptr<WakeupMessageQueue> wakeLockQueue = makeWakelockFMQ(kQueueSize);

    ::android::sp<ISensorsCallback> callback = new SensorsCallback();

    HalProxy proxy(subHals);

    // Initialize for the injectSensorData call so callback postEvents is valid

    proxy.initialize(*eventQueue->getDesc(), *wakeLockQueue->getDesc(), callback);



    // For testing proxy.injectSensorData properly

    proxy.setOperationMode(OperationMode::DATA_INJECTION);



    // kNumSubHalsInt32 index is one off the end of mSubHalList in proxy object

    EXPECT_EQ(proxy.activate(0x00000001 | (kNumSubHalsInt32 << 24), true), Result::BAD_VALUE);

    EXPECT_EQ(proxy.batch(0x00000001 | (kNumSubHalsInt32 << 24), 0, 0), Result::BAD_VALUE);

    EXPECT_EQ(proxy.flush(0x00000001 | (kNumSubHalsInt32 << 24)), Result::BAD_VALUE);

    Event event;

    event.sensorHandle = 0x00000001 | (kNumSubHalsInt32 << 24);

    EXPECT_EQ(proxy.injectSensorData(event), Result::BAD_VALUE);

}



// Helper implementations follow

void testSensorsListFromProxyAndSubHal(const std::vector<SensorInfo>& proxySensorsList,

                                       const std::vector<SensorInfo>& subHalSensorsList) {