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
diff --git a/services/audiopolicy/service/AudioPolicyEffects.cpp b/services/audiopolicy/service/AudioPolicyEffects.cpp
index eed545e..9a28137 100644
--- a/services/audiopolicy/service/AudioPolicyEffects.cpp
+++ b/services/audiopolicy/service/AudioPolicyEffects.cpp
@@ -374,7 +374,7 @@
// 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 @@
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);
- } 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);
- } 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);
- } 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;
- } else {
- *(bool *)((char *)param + pos) = true;
+ pos = growParamSize(param, sizeof(short), curSize, totSize);
+ if (pos == 0) {
+ goto exit;
}
- ALOGV("readParamValue() reading bool %s",*(bool *)((char *)param + pos) ? "true" : "false");
- return sizeof(bool);
+ *(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) {
+ 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) {
+ 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) {
+ 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 *)(*param + pos) = false;
+ }
+ 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);
+ strncpy(*param + *curSize, node->value, len);
*curSize += len;
- param[*curSize] = '\0';
- ALOGV("readParamValue() reading string %s", param + *curSize - len);
- return len;
+ (*param)[*curSize] = '\0';
+ ALOGV("readParamValue() reading string %s", *param + *curSize - len);
+ } else {
+ ALOGW("readParamValue() unknown param type %s", node->name);
}
- ALOGW("readParamValue() unknown param type %s", node->name);
- return 0;
+exit:
+ return len;
}
effect_param_t *AudioPolicyEffects::loadEffectParameter(cnode *root)
@@ -445,6 +472,12 @@
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 @@
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 @@
}
}
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 @@
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 @@
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 @@
return fx_param;
error:
- delete fx_param;
+ free(fx_param);
return NULL;
}
@@ -507,11 +545,9 @@
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);
}
- params.add(param);
node = node->next;
}
}
@@ -529,6 +565,7 @@
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);