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

Commit 138ed179 authored by Eric Laurent's avatar Eric Laurent
Browse files

audio policy: fix issues in effect parameters parsing 

Fix several issues in AudioPolicyEffects.cpp
- Fix old bug in growParamSize() that should take a pointer
to the address of the parameter structure because it can
modify it by calling realloc()
- Fix warnings reported by clang static analyzer
- Add checks on memory allocations

Bug: 26938281

Change-Id: Id0bfa64371d95356d9fc308c6ea9c74e10ab1be0
parent e3b06f27

services/audiopolicy/service/AudioPolicyEffects.cpp

+75 −38
Original line number Diff line number Diff line
@@ -374,7 +374,7 @@ audio_stream_type_t AudioPolicyEffects::streamNameToEnum(const char *name) 
// Audio Effect Config parser

// ----------------------------------------------------------------------------



size_t AudioPolicyEffects::growParamSize(char *param,

size_t AudioPolicyEffects::growParamSize(char **param,

                                         size_t size,

                                         size_t *curSize,

                                         size_t *totSize)
@@ -386,55 +386,82 @@ size_t AudioPolicyEffects::growParamSize(char *param, 
        while (pos + size > *totSize) {

            *totSize += ((*totSize + 7) / 8) * 4;

        }

        param = (char *)realloc(param, *totSize);

        *param = (char *)realloc(*param, *totSize);

        if (*param == NULL) {

            ALOGE("%s realloc error for size %zu", __func__, *totSize);

            return 0;

        }

    }

    *curSize = pos + size;

    return pos;

}





size_t AudioPolicyEffects::readParamValue(cnode *node,

                                          char *param,

                                          char **param,

                                          size_t *curSize,

                                          size_t *totSize)

{

    size_t len = 0;

    size_t pos;



    if (strncmp(node->name, SHORT_TAG, sizeof(SHORT_TAG) + 1) == 0) {

        size_t pos = growParamSize(param, sizeof(short), curSize, totSize);

        *(short *)((char *)param + pos) = (short)atoi(node->value);

        ALOGV("readParamValue() reading short %d", *(short *)((char *)param + pos));

        return sizeof(short);

        pos = growParamSize(param, sizeof(short), curSize, totSize);

        if (pos == 0) {

            goto exit;

        }

        *(short *)(*param + pos) = (short)atoi(node->value);

        ALOGV("readParamValue() reading short %d", *(short *)(*param + pos));

        len = sizeof(short);

    } else if (strncmp(node->name, INT_TAG, sizeof(INT_TAG) + 1) == 0) {

        size_t pos = growParamSize(param, sizeof(int), curSize, totSize);

        *(int *)((char *)param + pos) = atoi(node->value);

        ALOGV("readParamValue() reading int %d", *(int *)((char *)param + pos));

        return sizeof(int);

        pos = growParamSize(param, sizeof(int), curSize, totSize);

        if (pos == 0) {

            goto exit;

        }

        *(int *)(*param + pos) = atoi(node->value);

        ALOGV("readParamValue() reading int %d", *(int *)(*param + pos));

        len = sizeof(int);

    } else if (strncmp(node->name, FLOAT_TAG, sizeof(FLOAT_TAG) + 1) == 0) {

        size_t pos = growParamSize(param, sizeof(float), curSize, totSize);

        *(float *)((char *)param + pos) = (float)atof(node->value);

        ALOGV("readParamValue() reading float %f",*(float *)((char *)param + pos));

        return sizeof(float);

        pos = growParamSize(param, sizeof(float), curSize, totSize);

        if (pos == 0) {

            goto exit;

        }

        *(float *)(*param + pos) = (float)atof(node->value);

        ALOGV("readParamValue() reading float %f",*(float *)(*param + pos));

        len = sizeof(float);

    } else if (strncmp(node->name, BOOL_TAG, sizeof(BOOL_TAG) + 1) == 0) {

        size_t pos = growParamSize(param, sizeof(bool), curSize, totSize);

        if (strncmp(node->value, "false", strlen("false") + 1) == 0) {

            *(bool *)((char *)param + pos) = false;

        pos = growParamSize(param, sizeof(bool), curSize, totSize);

        if (pos == 0) {

            goto exit;

        }

        if (strncmp(node->value, "true", strlen("true") + 1) == 0) {

            *(bool *)(*param + pos) = true;

        } else {

            *(bool *)((char *)param + pos) = true;

            *(bool *)(*param + pos) = false;

        }

        ALOGV("readParamValue() reading bool %s",*(bool *)((char *)param + pos) ? "true" : "false");

        return sizeof(bool);

        ALOGV("readParamValue() reading bool %s",

              *(bool *)(*param + pos) ? "true" : "false");

        len = sizeof(bool);

    } else if (strncmp(node->name, STRING_TAG, sizeof(STRING_TAG) + 1) == 0) {

        size_t len = strnlen(node->value, EFFECT_STRING_LEN_MAX);

        len = strnlen(node->value, EFFECT_STRING_LEN_MAX);

        if (*curSize + len + 1 > *totSize) {

            *totSize = *curSize + len + 1;

            param = (char *)realloc(param, *totSize);

            *param = (char *)realloc(*param, *totSize);

            if (*param == NULL) {

                len = 0;

                ALOGE("%s realloc error for string len %zu", __func__, *totSize);

                goto exit;

            }

        strncpy(param + *curSize, node->value, len);

        *curSize += len;

        param[*curSize] = '\0';

        ALOGV("readParamValue() reading string %s", param + *curSize - len);

        return len;

        }

        strncpy(*param + *curSize, node->value, len);

        *curSize += len;

        (*param)[*curSize] = '\0';

        ALOGV("readParamValue() reading string %s", *param + *curSize - len);

    } else {

        ALOGW("readParamValue() unknown param type %s", node->name);

    return 0;

    }

exit:

    return len;

}



effect_param_t *AudioPolicyEffects::loadEffectParameter(cnode *root)
@@ -445,6 +472,12 @@ effect_param_t *AudioPolicyEffects::loadEffectParameter(cnode *root) 
    size_t totSize = sizeof(effect_param_t) + 2 * sizeof(int);

    effect_param_t *fx_param = (effect_param_t *)malloc(totSize);



    if (fx_param == NULL) {

        ALOGE("%s malloc error for effect structure of size %zu",

              __func__, totSize);

        return NULL;

    }



    param = config_find(root, PARAM_TAG);

    value = config_find(root, VALUE_TAG);

    if (param == NULL && value == NULL) {
@@ -453,8 +486,10 @@ effect_param_t *AudioPolicyEffects::loadEffectParameter(cnode *root) 
        if (param != NULL) {

            // Note: that a pair of random strings is read as 0 0

            int *ptr = (int *)fx_param->data;

#if LOG_NDEBUG == 0

            int *ptr2 = (int *)((char *)param + sizeof(effect_param_t));

            ALOGW("loadEffectParameter() ptr %p ptr2 %p", ptr, ptr2);

            ALOGV("loadEffectParameter() ptr %p ptr2 %p", ptr, ptr2);

#endif

            *ptr++ = atoi(param->name);

            *ptr = atoi(param->value);

            fx_param->psize = sizeof(int);
@@ -463,7 +498,8 @@ effect_param_t *AudioPolicyEffects::loadEffectParameter(cnode *root) 
        }

    }

    if (param == NULL || value == NULL) {

        ALOGW("loadEffectParameter() invalid parameter description %s", root->name);

        ALOGW("loadEffectParameter() invalid parameter description %s",

              root->name);

        goto error;

    }
@@ -471,7 +507,8 @@ effect_param_t *AudioPolicyEffects::loadEffectParameter(cnode *root) 
    param = param->first_child;

    while (param) {

        ALOGV("loadEffectParameter() reading param of type %s", param->name);

        size_t size = readParamValue(param, (char *)fx_param, &curSize, &totSize);

        size_t size =

                readParamValue(param, (char **)&fx_param, &curSize, &totSize);

        if (size == 0) {

            goto error;

        }
@@ -486,7 +523,8 @@ effect_param_t *AudioPolicyEffects::loadEffectParameter(cnode *root) 
    value = value->first_child;

    while (value) {

        ALOGV("loadEffectParameter() reading value of type %s", value->name);

        size_t size = readParamValue(value, (char *)fx_param, &curSize, &totSize);

        size_t size =

                readParamValue(value, (char **)&fx_param, &curSize, &totSize);

        if (size == 0) {

            goto error;

        }
@@ -497,7 +535,7 @@ effect_param_t *AudioPolicyEffects::loadEffectParameter(cnode *root) 
    return fx_param;



error:

    delete fx_param;

    free(fx_param);

    return NULL;

}
@@ -507,11 +545,9 @@ void AudioPolicyEffects::loadEffectParameters(cnode *root, Vector <effect_param_ 
    while (node) {

        ALOGV("loadEffectParameters() loading param %s", node->name);

        effect_param_t *param = loadEffectParameter(node);

        if (param == NULL) {

            node = node->next;

            continue;

        }

        if (param != NULL) {

            params.add(param);

        }

        node = node->next;

    }

}
@@ -529,6 +565,7 @@ AudioPolicyEffects::EffectDescVector *AudioPolicyEffects::loadEffectConfig( 
    EffectDescVector *desc = new EffectDescVector();

    while (node) {

        size_t i;



        for (i = 0; i < effects.size(); i++) {

            if (strncmp(effects[i]->mName, node->name, EFFECT_STRING_LEN_MAX) == 0) {

                ALOGV("loadEffectConfig() found effect %s in list", node->name);

services/audiopolicy/service/AudioPolicyEffects.h

+2 −2
Original line number Diff line number Diff line
@@ -171,10 +171,10 @@ private: 
    void loadEffectParameters(cnode *root, Vector <effect_param_t *>& params);

    effect_param_t *loadEffectParameter(cnode *root);

    size_t readParamValue(cnode *node,

                          char *param,

                          char **param,

                          size_t *curSize,

                          size_t *totSize);

    size_t growParamSize(char *param,

    size_t growParamSize(char **param,

                         size_t size,

                         size_t *curSize,

                         size_t *totSize);