Merge "MH2 | Check that subhal index is in range"

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<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);
 }
 
 /**
@@ -115,6 +128,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);
 }
@@ -188,11 +204,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));
 }
 
@@ -206,6 +228,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);
     }
@@ -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<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) {
@@ -603,11 +632,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

@@ -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

@@ -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) {