diff --git a/sensors/2.0/multihal/HalProxy.cpp b/sensors/2.0/multihal/HalProxy.cpp index 43e3af658c..b78806ab6b 100644 --- a/sensors/2.0/multihal/HalProxy.cpp +++ b/sensors/2.0/multihal/HalProxy.cpp @@ -44,6 +44,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. * @@ -52,8 +54,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(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(sensorHandle >> kBitsAfterSubHalIndex); } /** @@ -115,6 +128,9 @@ Return HalProxy::setOperationMode(OperationMode mode) { } Return HalProxy::activate(int32_t sensorHandle, bool enabled) { + if (!isSubHalIndexValid(sensorHandle)) { + return Result::BAD_VALUE; + } return getSubHalForSensorHandle(sensorHandle) ->activate(clearSubHalIndex(sensorHandle), enabled); } @@ -188,11 +204,17 @@ Return HalProxy::initialize( Return 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 HalProxy::flush(int32_t sensorHandle) { + if (!isSubHalIndexValid(sensorHandle)) { + return Result::BAD_VALUE; + } return getSubHalForSensorHandle(sensorHandle)->flush(clearSubHalIndex(sensorHandle)); } @@ -206,6 +228,9 @@ Return 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); } @@ -375,7 +400,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; } @@ -588,8 +613,12 @@ void HalProxy::setDirectChannelFlags(SensorInfo* sensorInfo, ISensorsSubHal* sub } } -ISensorsSubHal* HalProxy::getSubHalForSensorHandle(uint32_t sensorHandle) { - return mSubHalList[static_cast(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& events, size_t n) { @@ -603,11 +632,11 @@ size_t HalProxy::countNumWakeupEvents(const std::vector& 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; } diff --git a/sensors/2.0/multihal/include/HalProxy.h b/sensors/2.0/multihal/include/HalProxy.h index aee63b1f23..b1dd737025 100644 --- a/sensors/2.0/multihal/include/HalProxy.h +++ b/sensors/2.0/multihal/include/HalProxy.h @@ -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 mSensors; + std::map mSensors; //! Map of the dynamic sensors that have been added to halproxy. - std::map mDynamicSensors; + std::map 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); }; /** diff --git a/sensors/2.0/multihal/tests/HalProxy_test.cpp b/sensors/2.0/multihal/tests/HalProxy_test.cpp index 040e8c2ca4..75a4c22d11 100644 --- a/sensors/2.0/multihal/tests/HalProxy_test.cpp +++ b/sensors/2.0/multihal/tests/HalProxy_test.cpp @@ -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(kNumSubHals); + std::vector subHalObjs(kNumSubHals); + std::vector subHals; + for (const auto& subHal : subHalObjs) { + subHals.push_back((ISensorsSubHal*)(&subHal)); + } + + std::unique_ptr eventQueue = makeEventFMQ(kQueueSize); + std::unique_ptr wakeLockQueue = makeWakelockFMQ(kQueueSize); + ::android::sp 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& proxySensorsList, const std::vector& subHalSensorsList) {