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 5b06c33aa5..1b37cbd6fd 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 impl { 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::checkVendorMixedPropValue(const VehiclePropValue& } 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 @@ StatusCode DefaultVehicleHal::set(const VehiclePropValue& propValue) { 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 ecdb9a84a0..af29143bc2 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 @@ class DefaultVehicleHal : public VehicleHal { // 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 bbefce733a..891d8d60e3 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::VehiclePropValue; 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 @@ TEST_F(DefaultVhalImplTest, testDump) { 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 {}; @@ -463,19 +482,19 @@ class DefaultVhalImplSetPropRangeTest : public DefaultVhalImplTest, std::vector GenSetPropRangeParams() { std::vector 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});