Add input checks for Default VHAL.
Test: atest android.hardware.automotive.vehicle@2.0-default-impl-unit-tests
Bug: 188204722
Change-Id: I5f7084796a62c7e53a00f488fafb1026a182ce0c
diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultVehicleHal.cpp b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultVehicleHal.cpp
index a6e2c8f..f410f4c 100644
--- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultVehicleHal.cpp
+++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultVehicleHal.cpp
@@ -15,6 +15,7 @@
*/
#define LOG_TAG "DefaultVehicleHal_v2_0"
+#include <assert.h>
#include <utils/Log.h>
#include <utils/SystemClock.h>
#include <vhal_v2_0/RecurrentTimer.h>
@@ -72,6 +73,173 @@
return mVehicleClient->dump(fd, options);
}
+StatusCode DefaultVehicleHal::checkPropValue(const VehiclePropValue& value,
+ const VehiclePropConfig* config) {
+ int32_t property = value.prop;
+ VehiclePropertyType type = getPropType(property);
+ switch (type) {
+ case VehiclePropertyType::BOOLEAN:
+ case VehiclePropertyType::INT32:
+ if (value.value.int32Values.size() != 1) {
+ return StatusCode::INVALID_ARG;
+ }
+ break;
+ case VehiclePropertyType::INT32_VEC:
+ if (value.value.int32Values.size() < 1) {
+ return StatusCode::INVALID_ARG;
+ }
+ break;
+ case VehiclePropertyType::INT64:
+ if (value.value.int64Values.size() != 1) {
+ return StatusCode::INVALID_ARG;
+ }
+ break;
+ case VehiclePropertyType::INT64_VEC:
+ if (value.value.int64Values.size() < 1) {
+ return StatusCode::INVALID_ARG;
+ }
+ break;
+ case VehiclePropertyType::FLOAT:
+ if (value.value.floatValues.size() != 1) {
+ return StatusCode::INVALID_ARG;
+ }
+ break;
+ case VehiclePropertyType::FLOAT_VEC:
+ if (value.value.floatValues.size() < 1) {
+ return StatusCode::INVALID_ARG;
+ }
+ break;
+ case VehiclePropertyType::BYTES:
+ // We allow setting an empty bytes array.
+ break;
+ case VehiclePropertyType::STRING:
+ // We allow setting an empty string.
+ break;
+ case VehiclePropertyType::MIXED:
+ if (getPropGroup(property) == VehiclePropertyGroup::VENDOR) {
+ // We only checks vendor mixed properties.
+ return checkVendorMixedPropValue(value, config);
+ }
+ break;
+ default:
+ ALOGW("Unknown property type: %d", type);
+ return StatusCode::INVALID_ARG;
+ }
+ return StatusCode::OK;
+}
+
+StatusCode DefaultVehicleHal::checkVendorMixedPropValue(const VehiclePropValue& value,
+ const VehiclePropConfig* config) {
+ auto configArray = config->configArray;
+ // configArray[0], 1 indicates the property has a String value, we allow the string value to
+ // be empty.
+
+ size_t int32Count = 0;
+ // configArray[1], 1 indicates the property has a Boolean value.
+ if (configArray[1] == 1) {
+ int32Count++;
+ }
+ // configArray[2], 1 indicates the property has an Integer value.
+ if (configArray[2] == 1) {
+ int32Count++;
+ }
+ // configArray[3], the number indicates the size of Integer[] in the property.
+ int32Count += static_cast<size_t>(configArray[3]);
+ if (value.value.int32Values.size() != int32Count) {
+ return StatusCode::INVALID_ARG;
+ }
+
+ size_t int64Count = 0;
+ // configArray[4], 1 indicates the property has a Long value.
+ if (configArray[4] == 1) {
+ int64Count++;
+ }
+ // configArray[5], the number indicates the size of Long[] in the property.
+ int64Count += static_cast<size_t>(configArray[5]);
+ if (value.value.int64Values.size() != int64Count) {
+ return StatusCode::INVALID_ARG;
+ }
+
+ size_t floatCount = 0;
+ // configArray[6], 1 indicates the property has a Float value.
+ if (configArray[6] == 1) {
+ floatCount++;
+ }
+ // configArray[7], the number indicates the size of Float[] in the property.
+ floatCount += static_cast<size_t>(configArray[7]);
+ if (value.value.floatValues.size() != floatCount) {
+ return StatusCode::INVALID_ARG;
+ }
+
+ // configArray[8], the number indicates the size of byte[] in the property.
+ if (configArray[8] != 0 && value.value.bytes.size() != static_cast<size_t>(configArray[8])) {
+ return StatusCode::INVALID_ARG;
+ }
+ return StatusCode::OK;
+}
+
+StatusCode DefaultVehicleHal::checkValueRange(const VehiclePropValue& value,
+ const VehiclePropConfig* config) {
+ int32_t property = value.prop;
+ VehiclePropertyType type = getPropType(property);
+ const VehicleAreaConfig* areaConfig;
+ if (isGlobalProp(property)) {
+ if (config->areaConfigs.size() == 0) {
+ return StatusCode::OK;
+ }
+ areaConfig = &(config->areaConfigs[0]);
+ } else {
+ for (auto& c : config->areaConfigs) {
+ // areaId might contain multiple areas.
+ if (c.areaId & value.areaId) {
+ areaConfig = &c;
+ break;
+ }
+ }
+ }
+ switch (type) {
+ case VehiclePropertyType::INT32:
+ if (areaConfig->minInt32Value == 0 && areaConfig->maxInt32Value == 0) {
+ break;
+ }
+ // We already checked this in checkPropValue.
+ assert(value.value.int32Values.size() > 0);
+ if (value.value.int32Values[0] < areaConfig->minInt32Value ||
+ value.value.int32Values[0] > areaConfig->maxInt32Value) {
+ return StatusCode::INVALID_ARG;
+ }
+ break;
+ case VehiclePropertyType::INT64:
+ if (areaConfig->minInt64Value == 0 && areaConfig->maxInt64Value == 0) {
+ break;
+ }
+ // We already checked this in checkPropValue.
+ assert(value.value.int64Values.size() > 0);
+ if (value.value.int64Values[0] < areaConfig->minInt64Value ||
+ value.value.int64Values[0] > areaConfig->maxInt64Value) {
+ return StatusCode::INVALID_ARG;
+ }
+ break;
+ case VehiclePropertyType::FLOAT:
+ if (areaConfig->minFloatValue == 0 && areaConfig->maxFloatValue == 0) {
+ break;
+ }
+ // We already checked this in checkPropValue.
+ assert(value.value.floatValues.size() > 0);
+ if (value.value.floatValues[0] < areaConfig->minFloatValue ||
+ value.value.floatValues[0] > areaConfig->maxFloatValue) {
+ return StatusCode::INVALID_ARG;
+ }
+ break;
+ default:
+ // We don't check the rest of property types. Additional logic needs to be added if
+ // required for real implementation. E.g., you might want to enforce the range
+ // checks on vector as well or you might want to check the range for mixed property.
+ break;
+ }
+ return StatusCode::OK;
+}
+
StatusCode DefaultVehicleHal::set(const VehiclePropValue& propValue) {
if (propValue.status != VehiclePropertyStatus::AVAILABLE) {
// Android side cannot set property status - this value is the
@@ -79,12 +247,26 @@
// its underlying hardware
return StatusCode::INVALID_ARG;
}
- auto currentPropValue = mPropStore->readValueOrNull(propValue);
- if (currentPropValue == nullptr) {
+ int32_t property = propValue.prop;
+ const VehiclePropConfig* config = mPropStore->getConfigOrNull(property);
+ if (config == nullptr) {
+ ALOGW("no config for prop 0x%x", property);
return StatusCode::INVALID_ARG;
}
- if (currentPropValue->status != VehiclePropertyStatus::AVAILABLE) {
+ auto status = checkPropValue(propValue, config);
+ if (status != StatusCode::OK) {
+ ALOGW("invalid property value: %s", toString(propValue).c_str());
+ return status;
+ }
+ status = checkValueRange(propValue, config);
+ if (status != StatusCode::OK) {
+ ALOGW("property value out of range: %s", toString(propValue).c_str());
+ return status;
+ }
+
+ auto currentPropValue = mPropStore->readValueOrNull(propValue);
+ if (currentPropValue && currentPropValue->status != VehiclePropertyStatus::AVAILABLE) {
// do not allow Android side to set() a disabled/error property
return StatusCode::NOT_AVAILABLE;
}
@@ -151,6 +333,14 @@
if (!isContinuousProperty(property)) {
return StatusCode::INVALID_ARG;
}
+
+ // If the config does not exist, isContinuousProperty should return false.
+ const VehiclePropConfig* config = mPropStore->getConfigOrNull(property);
+ if (sampleRate < config->minSampleRate || sampleRate > config->maxSampleRate) {
+ ALOGW("sampleRate out of range");
+ return StatusCode::INVALID_ARG;
+ }
+
mRecurrentTimer.registerRecurrentEvent(hertzToNanoseconds(sampleRate), property);
return StatusCode::OK;
}
diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultVehicleHal.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultVehicleHal.h
index cc66c0f..2a0d430 100644
--- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultVehicleHal.h
+++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultVehicleHal.h
@@ -61,6 +61,16 @@
virtual void onPropertyValue(const VehiclePropValue& value, bool updateStatus);
// Returns a lambda that could be used in mRecurrentTimer.
RecurrentTimer::Action getTimerAction();
+ // Check whether a propValue is valid according to its type.
+ StatusCode checkPropValue(const VehiclePropValue& propValue, const VehiclePropConfig* config);
+ // Check whether the property value is within the range according to area config.
+ StatusCode checkValueRange(const VehiclePropValue& propValue, const VehiclePropConfig* config);
+
+ private:
+ // Check whether a vendor mixed value property is valid according to its config array.
+ // See 'VehiclePropertyType' documentation in 'types.hal' for detail.
+ StatusCode checkVendorMixedPropValue(const VehiclePropValue& value,
+ const VehiclePropConfig* config);
};
} // namespace impl
diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/tests/DefaultVhalImpl_test.cpp b/automotive/vehicle/2.0/default/impl/vhal_v2_0/tests/DefaultVhalImpl_test.cpp
index e943e67..a2ec0dd 100644
--- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/tests/DefaultVhalImpl_test.cpp
+++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/tests/DefaultVhalImpl_test.cpp
@@ -18,6 +18,7 @@
#include <gtest/gtest.h>
#include <sys/mman.h>
#include <vhal_v2_0/ConcurrentQueue.h>
+#include <vhal_v2_0/DefaultConfig.h>
#include <vhal_v2_0/DefaultVehicleConnector.h>
#include <vhal_v2_0/DefaultVehicleHal.h>
#include <vhal_v2_0/PropertyUtils.h>
@@ -40,6 +41,8 @@
using ::android::hardware::automotive::vehicle::V2_0::VehiclePropValuePool;
using ::android::hardware::automotive::vehicle::V2_0::impl::DefaultVehicleConnector;
using ::android::hardware::automotive::vehicle::V2_0::impl::DefaultVehicleHal;
+using ::android::hardware::automotive::vehicle::V2_0::impl::HVAC_LEFT;
+using ::android::hardware::automotive::vehicle::V2_0::impl::kMixedTypePropertyForTest;
using VehiclePropValuePtr = recyclable_ptr<VehiclePropValue>;
@@ -194,6 +197,32 @@
EXPECT_EQ("My Vehicle", gotValue->value.stringValue);
}
+TEST_F(DefaultVhalImplTest, testSetMixed) {
+ VehiclePropValue value;
+ value.prop = kMixedTypePropertyForTest;
+ // mixed prop.
+ // .configArray = {1, 1, 0, 2, 0, 0, 1, 0, 0}
+ // 1 string, 1 int, 0 bool, 2 ints, 0 int64, 0 int64s, 1 float, 0 floats, 0 bytes
+ value.value.stringValue = "test";
+ value.value.int32Values.resize(3);
+ value.value.int32Values[0] = 1;
+ value.value.int32Values[1] = 2;
+ value.value.int32Values[2] = 3;
+ value.value.floatValues.resize(1);
+ value.value.floatValues[0] = 1.0f;
+
+ StatusCode status = mHal->set(value);
+ EXPECT_EQ(StatusCode::OK, status);
+
+ auto gotValue = mHal->get(value, &status);
+ EXPECT_EQ(StatusCode::OK, status);
+ EXPECT_EQ("test", gotValue->value.stringValue);
+ EXPECT_EQ(1, gotValue->value.int32Values[0]);
+ EXPECT_EQ(2, gotValue->value.int32Values[1]);
+ EXPECT_EQ(3, gotValue->value.int32Values[2]);
+ EXPECT_EQ(1.0f, gotValue->value.floatValues[0]);
+}
+
TEST_F(DefaultVhalImplTest, testSetUnknownProperty) {
VehiclePropValue value;
value.prop = 0;
@@ -247,6 +276,13 @@
EXPECT_EQ(StatusCode::INVALID_ARG, mHal->subscribe(toInt(VehicleProperty::INFO_MAKE), 10));
}
+TEST_F(DefaultVhalImplTest, testSubscribeSampleRateOutOfRange) {
+ EXPECT_EQ(StatusCode::INVALID_ARG,
+ mHal->subscribe(toInt(VehicleProperty::PERF_VEHICLE_SPEED), 10.1));
+ EXPECT_EQ(StatusCode::INVALID_ARG,
+ mHal->subscribe(toInt(VehicleProperty::PERF_VEHICLE_SPEED), 0.5));
+}
+
TEST_F(DefaultVhalImplTest, testUnsubscribe) {
auto status = mHal->subscribe(toInt(VehicleProperty::PERF_VEHICLE_SPEED), 10);
EXPECT_EQ(StatusCode::OK, status);
@@ -300,4 +336,152 @@
EXPECT_NE(std::string::npos, std::string(buf).find(infoMake));
}
+class DefaultVhalImplSetInvalidPropTest : public DefaultVhalImplTest,
+ public testing::WithParamInterface<VehiclePropValue> {};
+
+std::vector<VehiclePropValue> GenSetInvalidPropParams() {
+ std::vector<VehiclePropValue> props;
+ // int prop with no value.
+ VehiclePropValue intProp = {.prop = toInt(VehicleProperty::INFO_MODEL_YEAR)};
+ props.push_back(intProp);
+
+ // int prop with more than one value.
+ VehiclePropValue intPropWithValues = {.prop = toInt(VehicleProperty::INFO_MODEL_YEAR)};
+ intPropWithValues.value.int32Values.resize(2);
+ props.push_back(intPropWithValues);
+
+ // int vec prop with no value.
+ VehiclePropValue intVecProp = {.prop = toInt(VehicleProperty::INFO_FUEL_TYPE)};
+ props.push_back(intVecProp);
+
+ // int64 prop with no value.
+ VehiclePropValue int64Prop = {.prop = toInt(VehicleProperty::EPOCH_TIME)};
+ props.push_back(int64Prop);
+
+ // int64 prop with more than one value.
+ VehiclePropValue int64PropWithValues = {.prop = toInt(VehicleProperty::EPOCH_TIME)};
+ int64PropWithValues.value.int64Values.resize(2);
+ props.push_back(int64PropWithValues);
+
+ // int64 vec prop with no value.
+ VehiclePropValue int64VecProp = {.prop = toInt(VehicleProperty::WHEEL_TICK)};
+ props.push_back(int64VecProp);
+
+ // float prop with no value.
+ VehiclePropValue floatProp = {.prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY)};
+ props.push_back(floatProp);
+
+ // float prop with more than one value.
+ VehiclePropValue floatPropWithValues = {.prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY)};
+ floatPropWithValues.value.floatValues.resize(2);
+ props.push_back(floatPropWithValues);
+
+ // float vec prop with no value.
+ VehiclePropValue floatVecProp = {
+ .prop = toInt(VehicleProperty::HVAC_TEMPERATURE_VALUE_SUGGESTION)};
+ props.push_back(floatVecProp);
+
+ // bool prop with no value.
+ VehiclePropValue boolProp = {
+ .prop = toInt(VehicleProperty::FUEL_CONSUMPTION_UNITS_DISTANCE_OVER_VOLUME)};
+ props.push_back(boolProp);
+
+ // bool prop with more than one value.
+ VehiclePropValue boolPropWithValues = {
+ .prop = toInt(VehicleProperty::FUEL_CONSUMPTION_UNITS_DISTANCE_OVER_VOLUME)};
+ boolPropWithValues.value.int32Values.resize(2);
+ props.push_back(boolPropWithValues);
+
+ // mixed prop.
+ // .configArray = {1, 1, 0, 2, 0, 0, 1, 0, 0}
+ // 1 string, 1 int, 0 bool, 2 ints, 0 int64, 0 int64s, 1 float, 0 floats, 0 bytes
+ VehiclePropValue mixedProp1 = {.prop = kMixedTypePropertyForTest};
+ // Expect 1 bool, and 2 ints, we only have 1 value.
+ mixedProp1.value.int32Values.resize(1);
+ mixedProp1.value.floatValues.resize(1);
+ props.push_back(mixedProp1);
+
+ VehiclePropValue mixedProp2 = {.prop = kMixedTypePropertyForTest};
+ mixedProp2.value.int32Values.resize(3);
+ // Missing float value.
+ mixedProp2.value.floatValues.resize(0);
+ props.push_back(mixedProp2);
+
+ return props;
+}
+
+TEST_P(DefaultVhalImplSetInvalidPropTest, testSetInvalidPropValue) {
+ VehiclePropValue value = GetParam();
+
+ StatusCode status = mHal->set(value);
+
+ EXPECT_EQ(StatusCode::INVALID_ARG, status);
+}
+
+INSTANTIATE_TEST_SUITE_P(DefaultVhalImplSetInvalidPropTests, DefaultVhalImplSetInvalidPropTest,
+ testing::ValuesIn(GenSetInvalidPropParams()));
+
+struct SetPropRangeTestCase {
+ std::string name;
+ VehiclePropValue prop;
+ StatusCode code;
+};
+
+class DefaultVhalImplSetPropRangeTest : public DefaultVhalImplTest,
+ public testing::WithParamInterface<SetPropRangeTestCase> {};
+
+std::vector<SetPropRangeTestCase> GenSetPropRangeParams() {
+ std::vector<SetPropRangeTestCase> tc;
+ VehiclePropValue intPropNormal = {.prop = toInt(VehicleProperty::HVAC_FAN_SPEED),
+ .areaId = HVAC_LEFT,
+ // min: 1, max: 7
+ .value.int32Values = {3}};
+ tc.push_back({"normal_case_int", intPropNormal, StatusCode::OK});
+
+ VehiclePropValue intPropSmall = {.prop = toInt(VehicleProperty::HVAC_FAN_SPEED),
+ .areaId = HVAC_LEFT,
+ // min: 1, max: 7
+ .value.int32Values = {0}};
+ tc.push_back({"normal_case_int_too_small", intPropSmall, StatusCode::INVALID_ARG});
+
+ VehiclePropValue intPropLarge = {.prop = toInt(VehicleProperty::HVAC_FAN_SPEED),
+ .areaId = HVAC_LEFT,
+ // min: 1, max: 7
+ .value.int32Values = {8}};
+ tc.push_back({"normal_case_int_too_large", intPropLarge, StatusCode::INVALID_ARG});
+
+ VehiclePropValue floatPropNormal = {.prop = toInt(VehicleProperty::HVAC_TEMPERATURE_SET),
+ .areaId = HVAC_LEFT,
+ // min: 16, max: 32
+ .value.floatValues = {26}};
+ tc.push_back({"normal_case_float", floatPropNormal, StatusCode::OK});
+ VehiclePropValue floatPropSmall = {.prop = toInt(VehicleProperty::HVAC_TEMPERATURE_SET),
+ .areaId = HVAC_LEFT,
+ // min: 16, max: 32
+ .value.floatValues = {15.5}};
+ tc.push_back({"normal_case_float_too_small", floatPropSmall, StatusCode::INVALID_ARG});
+ VehiclePropValue floatPropLarge = {.prop = toInt(VehicleProperty::HVAC_TEMPERATURE_SET),
+ .areaId = HVAC_LEFT,
+ // min: 16, max: 32
+ .value.floatValues = {32.6}};
+ tc.push_back({"normal_case_float_too_large", floatPropLarge, StatusCode::INVALID_ARG});
+
+ return tc;
+}
+
+TEST_P(DefaultVhalImplSetPropRangeTest, testSetPropRange) {
+ SetPropRangeTestCase tc = GetParam();
+
+ StatusCode status = mHal->set(tc.prop);
+
+ EXPECT_EQ(tc.code, status);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ DefaultVhalImplSetPropRangeTests, DefaultVhalImplSetPropRangeTest,
+ testing::ValuesIn(GenSetPropRangeParams()),
+ [](const testing::TestParamInfo<DefaultVhalImplSetPropRangeTest::ParamType>& info) {
+ return info.param.name;
+ });
+
} // namespace