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

Commit 873e3492 authored by Mikhail Naganov's avatar Mikhail Naganov Committed by Automerger Merge Worker
Browse files

audio: Add checks to HIDL -> effect_param_t conversion am: 4f110343 

Original change: https://googleplex-android-review.googlesource.com/c/platform/hardware/interfaces/+/19213829



Change-Id: I39e8cd8b1e70fd34bc683ae62aaa596a83c2973f
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 8321c7e7 4f110343

audio/effect/all-versions/default/Effect.cpp

+32 −11
Original line number Diff line number Diff line
@@ -238,12 +238,27 @@ void Effect::effectOffloadParamToHal(const EffectOffloadParameter& offload, 
}



// static

std::vector<uint8_t> Effect::parameterToHal(uint32_t paramSize, const void* paramData,

                                            uint32_t valueSize, const void** valueData) {

bool Effect::parameterToHal(uint32_t paramSize, const void* paramData, uint32_t valueSize,

                            const void** valueData, std::vector<uint8_t>* halParamBuffer) {

    constexpr size_t kMaxSize = EFFECT_PARAM_SIZE_MAX - sizeof(effect_param_t);

    if (paramSize > kMaxSize) {

        ALOGE("%s: Parameter size is too big: %" PRIu32, __func__, paramSize);

        return false;

    }

    size_t valueOffsetFromData = alignedSizeIn<uint32_t>(paramSize) * sizeof(uint32_t);

    if (valueOffsetFromData > kMaxSize) {

        ALOGE("%s: Aligned parameter size is too big: %zu", __func__, valueOffsetFromData);

        return false;

    }

    if (valueSize > kMaxSize - valueOffsetFromData) {

        ALOGE("%s: Value size is too big: %" PRIu32 ", max size is %zu", __func__, valueSize,

              kMaxSize - valueOffsetFromData);

        android_errorWriteLog(0x534e4554, "237291425");

        return false;

    }

    size_t halParamBufferSize = sizeof(effect_param_t) + valueOffsetFromData + valueSize;

    std::vector<uint8_t> halParamBuffer(halParamBufferSize, 0);

    effect_param_t* halParam = reinterpret_cast<effect_param_t*>(&halParamBuffer[0]);

    halParamBuffer->resize(halParamBufferSize, 0);

    effect_param_t* halParam = reinterpret_cast<effect_param_t*>(halParamBuffer->data());

    halParam->psize = paramSize;

    halParam->vsize = valueSize;

    memcpy(halParam->data, paramData, paramSize);
@@ -256,7 +271,7 @@ std::vector<uint8_t> Effect::parameterToHal(uint32_t paramSize, const void* para 
            *valueData = halParam->data + valueOffsetFromData;

        }

    }

    return halParamBuffer;

    return true;

}



Result Effect::analyzeCommandStatus(const char* commandName, const char* context, status_t status) {
@@ -314,11 +329,15 @@ Result Effect::getParameterImpl(uint32_t paramSize, const void* paramData, 
                                GetParameterSuccessCallback onSuccess) {

    // As it is unknown what method HAL uses for copying the provided parameter data,

    // it is safer to make sure that input and output buffers do not overlap.

    std::vector<uint8_t> halCmdBuffer =

        parameterToHal(paramSize, paramData, requestValueSize, nullptr);

    std::vector<uint8_t> halCmdBuffer;

    if (!parameterToHal(paramSize, paramData, requestValueSize, nullptr, &halCmdBuffer)) {

        return Result::INVALID_ARGUMENTS;

    }

    const void* valueData = nullptr;

    std::vector<uint8_t> halParamBuffer =

        parameterToHal(paramSize, paramData, replyValueSize, &valueData);

    std::vector<uint8_t> halParamBuffer;

    if (!parameterToHal(paramSize, paramData, replyValueSize, &valueData, &halParamBuffer)) {

        return Result::INVALID_ARGUMENTS;

    }

    uint32_t halParamBufferSize = halParamBuffer.size();



    return sendCommandReturningStatusAndData(
@@ -472,8 +491,10 @@ Result Effect::setConfigImpl(int commandCode, const char* commandName, const Eff 


Result Effect::setParameterImpl(uint32_t paramSize, const void* paramData, uint32_t valueSize,

                                const void* valueData) {

    std::vector<uint8_t> halParamBuffer =

        parameterToHal(paramSize, paramData, valueSize, &valueData);

    std::vector<uint8_t> halParamBuffer;

    if (!parameterToHal(paramSize, paramData, valueSize, &valueData, &halParamBuffer)) {

        return Result::INVALID_ARGUMENTS;

    }

    return sendCommandReturningStatus(EFFECT_CMD_SET_PARAM, "SET_PARAM", halParamBuffer.size(),

                                      &halParamBuffer[0]);

}

audio/effect/all-versions/default/Effect.h

+2 −2
Original line number Diff line number Diff line
@@ -211,8 +211,8 @@ struct Effect : public IEffect { 
                                             channel_config_t* halConfig);

    static void effectOffloadParamToHal(const EffectOffloadParameter& offload,

                                        effect_offload_param_t* halOffload);

    static std::vector<uint8_t> parameterToHal(uint32_t paramSize, const void* paramData,

                                               uint32_t valueSize, const void** valueData);

    static bool parameterToHal(uint32_t paramSize, const void* paramData, uint32_t valueSize,

                               const void** valueData, std::vector<uint8_t>* halParamBuffer);



    Result analyzeCommandStatus(const char* commandName, const char* context, status_t status);

    void getConfigImpl(int commandCode, const char* commandName, GetConfigCallback cb);

audio/effect/all-versions/vts/functional/VtsHalAudioEffectTargetTest.cpp

+17 −0
Original line number Diff line number Diff line
@@ -623,6 +623,23 @@ TEST_P(AudioEffectHidlTest, GetParameter) { 
    EXPECT_TRUE(ret.isOk());

}



TEST_P(AudioEffectHidlTest, GetParameterInvalidMaxReplySize) {

    description("Verify that GetParameter caps the maximum reply size");

    // Use a non-empty parameter to avoid being rejected by any earlier checks.

    hidl_vec<uint8_t> parameter;

    parameter.resize(16);

    // Use very large size to ensure that the service does not crash. Since parameters

    // are specific to each effect, and some effects may not have parameters at all,

    // simply checking the return value would not reveal an issue of using an uncapped value.

    const uint32_t veryLargeReplySize = std::numeric_limits<uint32_t>::max() - 100;

    Result retval = Result::OK;

    Return<void> ret =

            effect->getParameter(parameter, veryLargeReplySize,

                                 [&](Result r, const hidl_vec<uint8_t>&) { retval = r; });

    EXPECT_TRUE(ret.isOk());

    EXPECT_EQ(Result::INVALID_ARGUMENTS, retval);

}



TEST_P(AudioEffectHidlTest, GetSupportedConfigsForFeature) {

    description("Verify that GetSupportedConfigsForFeature does not crash");

    Return<void> ret = effect->getSupportedConfigsForFeature(