Add areaId check.
According to types.hal, a non-global properties should have areaConfigs
that define allowed areaIds. We need to check that the areaId is in
the areaConfigs list before setting the value to prevent unexpected
behavior.
Test: unit test.
Bug: 193831021
Change-Id: I90faf808aa6ac5278e99cf6313454515afaaca2f
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 5b06c33..1b37cbd 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
@@ -35,6 +35,23 @@
namespace {
constexpr std::chrono::nanoseconds kHeartBeatIntervalNs = 3s;
+
+const VehicleAreaConfig* getAreaConfig(const VehiclePropValue& propValue,
+ const VehiclePropConfig* config) {
+ if (isGlobalProp(propValue.prop)) {
+ if (config->areaConfigs.size() == 0) {
+ return nullptr;
+ }
+ return &(config->areaConfigs[0]);
+ } else {
+ for (auto& c : config->areaConfigs) {
+ if (c.areaId == propValue.areaId) {
+ return &c;
+ }
+ }
+ }
+ return nullptr;
+}
} // namespace
DefaultVehicleHal::DefaultVehicleHal(VehiclePropertyStore* propStore, VehicleHalClient* client)
@@ -184,24 +201,12 @@
}
StatusCode DefaultVehicleHal::checkValueRange(const VehiclePropValue& value,
- const VehiclePropConfig* config) {
+ const VehicleAreaConfig* areaConfig) {
+ if (areaConfig == nullptr) {
+ return StatusCode::OK;
+ }
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) {
@@ -259,12 +264,20 @@
ALOGW("no config for prop 0x%x", property);
return StatusCode::INVALID_ARG;
}
+ const VehicleAreaConfig* areaConfig = getAreaConfig(propValue, config);
+ if (!isGlobalProp(property) && areaConfig == nullptr) {
+ // Ignore areaId for global property. For non global property, check whether areaId is
+ // allowed. areaId must appear in areaConfig.
+ ALOGW("invalid area ID: 0x%x for prop 0x%x, not listed in config", propValue.areaId,
+ property);
+ return StatusCode::INVALID_ARG;
+ }
auto status = checkPropValue(propValue, config);
if (status != StatusCode::OK) {
ALOGW("invalid property value: %s", toString(propValue).c_str());
return status;
}
- status = checkValueRange(propValue, config);
+ status = checkValueRange(propValue, areaConfig);
if (status != StatusCode::OK) {
ALOGW("property value out of range: %s", toString(propValue).c_str());
return status;
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 ecdb9a8..af29143 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
@@ -64,7 +64,8 @@
// 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);
+ StatusCode checkValueRange(const VehiclePropValue& propValue,
+ const VehicleAreaConfig* areaConfig);
// Register the heart beat event to be sent every 3s. This is required to inform watch dog that
// VHAL is alive. Subclasses should always calls this function during onCreate.
void registerHeartBeatEvent();
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 bbefce7..891d8d6 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
@@ -44,6 +44,7 @@
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_ALL;
using ::android::hardware::automotive::vehicle::V2_0::impl::HVAC_LEFT;
using ::android::hardware::automotive::vehicle::V2_0::impl::kMixedTypePropertyForTest;
@@ -366,6 +367,24 @@
EXPECT_THAT(std::string(buf), HasSubstr(infoMake));
}
+TEST_F(DefaultVhalImplTest, testSetPropInvalidAreaId) {
+ VehiclePropValue propNormal = {.prop = toInt(VehicleProperty::HVAC_FAN_SPEED),
+ .areaId = HVAC_ALL,
+ .value.int32Values = {3}};
+ StatusCode status = mHal->set(propNormal);
+
+ EXPECT_EQ(StatusCode::OK, status);
+
+ // HVAC_FAN_SPEED only have HVAC_ALL area config and is not allowed to set by LEFT/RIGHT.
+ VehiclePropValue propWrongId = {.prop = toInt(VehicleProperty::HVAC_FAN_SPEED),
+ .areaId = HVAC_LEFT,
+ .value.int32Values = {3}};
+
+ status = mHal->set(propWrongId);
+
+ EXPECT_EQ(StatusCode::INVALID_ARG, status);
+}
+
class DefaultVhalImplSetInvalidPropTest : public DefaultVhalImplTest,
public testing::WithParamInterface<VehiclePropValue> {};
@@ -463,19 +482,19 @@
std::vector<SetPropRangeTestCase> GenSetPropRangeParams() {
std::vector<SetPropRangeTestCase> tc;
VehiclePropValue intPropNormal = {.prop = toInt(VehicleProperty::HVAC_FAN_SPEED),
- .areaId = HVAC_LEFT,
+ .areaId = HVAC_ALL,
// 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,
+ .areaId = HVAC_ALL,
// 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,
+ .areaId = HVAC_ALL,
// min: 1, max: 7
.value.int32Values = {8}};
tc.push_back({"normal_case_int_too_large", intPropLarge, StatusCode::INVALID_ARG});