Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 656882ac authored by Yu Shan's avatar Yu Shan
Browse files

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
parent e39d3942

automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultVehicleHal.cpp

+30 −17
Original line number Diff line number Diff line
@@ -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) {

    int32_t property = value.prop;

    VehiclePropertyType type = getPropType(property);

    const VehicleAreaConfig* areaConfig;

    if (isGlobalProp(property)) {

        if (config->areaConfigs.size() == 0) {

                                              const VehicleAreaConfig* areaConfig) {

    if (areaConfig == nullptr) {

        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;

            }

        }

    }

    int32_t property = value.prop;

    VehiclePropertyType type = getPropType(property);

    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;

automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultVehicleHal.h

+2 −1
Original line number Diff line number Diff line
@@ -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();

automotive/vehicle/2.0/default/impl/vhal_v2_0/tests/DefaultVhalImpl_test.cpp

+22 −3
Original line number Diff line number Diff line
@@ -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<VehiclePropValue> {};
@@ -463,19 +482,19 @@ class DefaultVhalImplSetPropRangeTest : public DefaultVhalImplTest, 
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});