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

Commit 82992565 authored by Shunkai Yao's avatar Shunkai Yao Committed by Cherrypicker Worker
Browse files

Add AIDL union tag checking before access 

And improve some parameter logging

Bug: 296954124
Test: Enable AIDL and test with YTM on Pixel
(cherry picked from https://android-review.googlesource.com/q/commit:b11b7fe1b13e8e0e98f376f28767bd2dee31beb7)
Merged-In: Ic3373a138b8aa02d6c283342b48765ba909855de
Change-Id: Ic3373a138b8aa02d6c283342b48765ba909855de
parent aa257ab1

media/libaudiohal/impl/EffectConversionHelperAidl.cpp

+53 −32
Original line number Diff line number Diff line
@@ -102,6 +102,8 @@ status_t EffectConversionHelperAidl::handleInit(uint32_t cmdSize __unused, 
                                                const void* pCmdData __unused, uint32_t* replySize,

                                                void* pReplyData) {

    if (!replySize || *replySize < sizeof(int) || !pReplyData) {

        ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,

              numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }
@@ -111,8 +113,10 @@ status_t EffectConversionHelperAidl::handleInit(uint32_t cmdSize __unused, 


status_t EffectConversionHelperAidl::handleSetParameter(uint32_t cmdSize, const void* pCmdData,

                                                        uint32_t* replySize, void* pReplyData) {

    if (cmdSize < sizeof(effect_param_t) || !pCmdData || !replySize ||

        *replySize < sizeof(int) || !pReplyData) {

    if (cmdSize < sizeof(effect_param_t) || !pCmdData || !replySize || *replySize < sizeof(int) ||

        !pReplyData) {

        ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,

              cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }
@@ -131,8 +135,8 @@ status_t EffectConversionHelperAidl::handleSetParameter(uint32_t cmdSize, const 
status_t EffectConversionHelperAidl::handleGetParameter(uint32_t cmdSize, const void* pCmdData,

                                                        uint32_t* replySize, void* pReplyData) {

    if (cmdSize < sizeof(effect_param_t) || !pCmdData || !replySize || !pReplyData) {

        ALOGE("%s illegal cmdSize %u pCmdData %p replySize %p replyData %p", __func__, cmdSize,

              pCmdData, replySize, pReplyData);

        ALOGE("%s illegal cmdSize %u pCmdData %p replySize %s replyData %p", __func__, cmdSize,

              pCmdData, numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }
@@ -159,21 +163,21 @@ status_t EffectConversionHelperAidl::handleSetConfig(uint32_t cmdSize, const voi 
                                                     uint32_t* replySize, void* pReplyData) {

    if (!replySize || *replySize != sizeof(int) || !pReplyData ||

        cmdSize != sizeof(effect_config_t)) {

        ALOGE("%s parameter invalid %u %p %p %p", __func__, cmdSize, pCmdData, replySize,

              pReplyData);

        ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,

              cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }



    effect_config_t* config = (effect_config_t*)pCmdData;

    Parameter::Common common = {

            .session = mCommon.session,

            .ioHandle = mCommon.ioHandle,

            .input =

                    VALUE_OR_RETURN_STATUS(::aidl::android::legacy2aidl_buffer_config_t_AudioConfig(

                            config->inputCfg, mIsInputStream)),

            .output =

                    VALUE_OR_RETURN_STATUS(::aidl::android::legacy2aidl_buffer_config_t_AudioConfig(

                            config->outputCfg, mIsInputStream)),

            .session = mCommon.session,

            .ioHandle = mCommon.ioHandle};

                            config->outputCfg, mIsInputStream))};



    State state;

    RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getState(&state)));
@@ -225,13 +229,19 @@ status_t EffectConversionHelperAidl::handleGetConfig(uint32_t cmdSize __unused, 
                                                     const void* pCmdData __unused,

                                                     uint32_t* replySize, void* pReplyData) {

    if (!replySize || *replySize != sizeof(effect_config_t) || !pReplyData) {

        ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);

        ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,

              numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }



    Parameter param;

    RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(

            Parameter::Id::make<Parameter::Id::commonTag>(Parameter::common), &param)));

    if (param.getTag() != Parameter::common) {

        *replySize = 0;

        ALOGW("%s no valid common tag return from HAL: %s", __func__, param.toString().c_str());

        return BAD_VALUE;

    }



    const auto& common = param.get<Parameter::common>();

    effect_config_t* pConfig = (effect_config_t*)pReplyData;
@@ -246,7 +256,8 @@ status_t EffectConversionHelperAidl::handleReset(uint32_t cmdSize __unused, 
                                                 const void* pCmdData __unused, uint32_t* replySize,

                                                 void* pReplyData) {

    if (!replySize || !pReplyData) {

        ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);

        ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,

              numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }
@@ -257,7 +268,8 @@ status_t EffectConversionHelperAidl::handleEnable(uint32_t cmdSize __unused, 
                                                  const void* pCmdData __unused,

                                                  uint32_t* replySize, void* pReplyData) {

    if (!replySize || !pReplyData) {

        ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);

        ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,

              numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }
@@ -268,7 +280,8 @@ status_t EffectConversionHelperAidl::handleDisable(uint32_t cmdSize __unused, 
                                                   const void* pCmdData __unused,

                                                   uint32_t* replySize, void* pReplyData) {

    if (!replySize || !pReplyData) {

        ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);

        ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,

              numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }
@@ -278,8 +291,8 @@ status_t EffectConversionHelperAidl::handleDisable(uint32_t cmdSize __unused, 
status_t EffectConversionHelperAidl::handleSetAudioSource(uint32_t cmdSize, const void* pCmdData,

                                                          uint32_t* replySize, void* pReplyData) {

    if (cmdSize != sizeof(uint32_t) || !pCmdData || !replySize || !pReplyData) {

        ALOGE("%s parameter invalid %u %p %p %p", __func__, cmdSize, pCmdData, replySize,

              pReplyData);

        ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,

              cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }

    if (!getDescriptor().common.flags.audioSourceIndication) {
@@ -298,8 +311,8 @@ status_t EffectConversionHelperAidl::handleSetAudioSource(uint32_t cmdSize, cons 
status_t EffectConversionHelperAidl::handleSetAudioMode(uint32_t cmdSize, const void* pCmdData,

                                                        uint32_t* replySize, void* pReplyData) {

    if (cmdSize != sizeof(uint32_t) || !pCmdData || !replySize || !pReplyData) {

        ALOGE("%s parameter invalid %u %p %p %p", __func__, cmdSize, pCmdData, replySize,

              pReplyData);

        ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,

              cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }

    if (!getDescriptor().common.flags.audioModeIndication) {
@@ -317,8 +330,8 @@ status_t EffectConversionHelperAidl::handleSetAudioMode(uint32_t cmdSize, const 
status_t EffectConversionHelperAidl::handleSetDevice(uint32_t cmdSize, const void* pCmdData,

                                                     uint32_t* replySize, void* pReplyData) {

    if (cmdSize != sizeof(uint32_t) || !pCmdData || !replySize || !pReplyData) {

        ALOGE("%s parameter invalid %u %p %p %p", __func__, cmdSize, pCmdData, replySize,

              pReplyData);

        ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,

              cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }

    if (!getDescriptor().common.flags.deviceIndication) {
@@ -354,21 +367,27 @@ status_t EffectConversionHelperAidl::handleSetVolume(uint32_t cmdSize, const voi 
    }



    constexpr uint32_t unityGain = 1 << 24;

    Parameter::VolumeStereo requestedVolume = {.left = (float)(*(uint32_t*)pCmdData) / unityGain,

                                      .right = (float)(*(uint32_t*)pCmdData + 1) / unityGain};



    uint32_t vl = *(uint32_t*)pCmdData;

    uint32_t vr = *((uint32_t*)pCmdData + 1);

    RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(

            mEffect->setParameter(Parameter::make<Parameter::volumeStereo>(requestedVolume))));

            mEffect->setParameter(Parameter::make<Parameter::volumeStereo>(Parameter::VolumeStereo(

                    {.left = (float)vl / unityGain, .right = (float)vr / unityGain})))));



    // get volume from effect and set if changed.

    // get volume from effect and set if changed, return the volume in command if HAL not return

    // correct parameter.

    Parameter::Id id = Parameter::Id::make<Parameter::Id::commonTag>(Parameter::volumeStereo);

    Parameter volParam;

    RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &volParam)));

    const status_t getParamStatus = statusTFromBinderStatus(mEffect->getParameter(id, &volParam));

    if (getParamStatus != OK || volParam.getTag() != Parameter::volumeStereo) {

        ALOGW("%s no valid volume return from HAL, status %d: %s, return volume in command",

              __func__, getParamStatus, volParam.toString().c_str());

    } else {

        Parameter::VolumeStereo appliedVolume = volParam.get<Parameter::volumeStereo>();

        vl = (uint32_t)(appliedVolume.left * unityGain);

        vr = (uint32_t)(appliedVolume.right * unityGain);

    }



    if (replySize && *replySize == 2 * sizeof(uint32_t) && pReplyData) {

        uint32_t vl = (uint32_t)(appliedVolume.left * unityGain);

        uint32_t vr = (uint32_t)(appliedVolume.right * unityGain);

        uint32_t vol_ret[2] = {vl, vr};

        memcpy(pReplyData, vol_ret, sizeof(vol_ret));

    }
@@ -378,8 +397,8 @@ status_t EffectConversionHelperAidl::handleSetVolume(uint32_t cmdSize, const voi 
status_t EffectConversionHelperAidl::handleSetOffload(uint32_t cmdSize, const void* pCmdData,

                                                      uint32_t* replySize, void* pReplyData) {

    if (cmdSize < sizeof(effect_offload_param_t) || !pCmdData || !replySize || !pReplyData) {

        ALOGE("%s parameter invalid %u %p %p %p", __func__, cmdSize, pCmdData, replySize,

              pReplyData);

        ALOGE("%s parameter invalid, cmdSize %u pCmdData %p replySize %s pReplyData %p", __func__,

              cmdSize, pCmdData, numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }

    effect_offload_param_t* offload = (effect_offload_param_t*)pCmdData;
@@ -411,7 +430,8 @@ status_t EffectConversionHelperAidl::handleVisualizerCapture(uint32_t cmdSize __ 
                                                             uint32_t* replySize,

                                                             void* pReplyData) {

    if (!replySize || !pReplyData) {

        ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);

        ALOGE("%s parameter invalid replySize %s pReplyData %p", __func__,

              numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }
@@ -431,7 +451,8 @@ status_t EffectConversionHelperAidl::handleVisualizerMeasure(uint32_t cmdSize __ 
                                                             uint32_t* replySize,

                                                             void* pReplyData) {

    if (!replySize || !pReplyData) {

        ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);

        ALOGE("%s parameter invalid, replySize %s pReplyData %p", __func__,

              numericPointerToString(replySize).c_str(), pReplyData);

        return BAD_VALUE;

    }

media/libaudiohal/impl/EffectConversionHelperAidl.h

+5 −0
Original line number Diff line number Diff line
@@ -72,6 +72,11 @@ class EffectConversionHelperAidl { 


    static constexpr int kDefaultframeCount = 0x100;



    template <typename T, typename = std::enable_if_t<std::is_arithmetic_v<T>>>

    static inline std::string numericPointerToString(T* pt) {

        return pt ? std::to_string(*pt) : "nullptr";

    }



    using AudioChannelLayout = aidl::android::media::audio::common::AudioChannelLayout;

    const aidl::android::media::audio::common::AudioConfig kDefaultAudioConfig = {

            .base = {.sampleRate = 44100,

media/libaudiohal/impl/effectsAidlConversion/AidlConversionDynamicsProcessing.cpp

+1 −1
Original line number Diff line number Diff line
@@ -227,7 +227,7 @@ AidlConversionDp::readChannelConfigFromParam(EffectParamReader& param) { 
    RETURN_IF_ERROR(param.readFromValue(&enable));



    return DynamicsProcessing::ChannelConfig(

            {.enable = VALUE_OR_RETURN(convertIntegral<bool>(enable)), .channel = channel});

            {.channel = channel, .enable = VALUE_OR_RETURN(convertIntegral<bool>(enable))});

}



ConversionResult<DynamicsProcessing::EqBandConfig>