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
diff --git a/sensors/2.0/multihal/HalProxy.cpp b/sensors/2.0/multihal/HalProxy.cpp
index fe2b98b..38a627f 100644
--- a/sensors/2.0/multihal/HalProxy.cpp
+++ b/sensors/2.0/multihal/HalProxy.cpp
@@ -42,6 +42,8 @@
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 @@
*
* @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::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::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 @@
}
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 @@
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 @@
}
}
-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 @@
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 26bc644..10fce43 100644
--- a/sensors/2.0/multihal/include/HalProxy.h
+++ b/sensors/2.0/multihal/include/HalProxy.h
@@ -124,7 +124,7 @@
*
* @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 @@
* 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 @@
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 @@
*
* @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 @@
*
* @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 040e8c2..75a4c22 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, 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) {