Hi Praveen, Ack (Test only).
Thanks, Quyen -----Original Message----- From: praveen.malv...@oracle.com [mailto:praveen.malv...@oracle.com] Sent: Thursday, November 26, 2015 7:11 PM To: hans.nordeb...@ericsson.com; nagendr...@oracle.com; quyen....@dektech.com.au Cc: opensaf-devel@lists.sourceforge.net Subject: [PATCH 1 of 1] amfd: fix amfd crash during attribute modification in compType v2[#1593] osaf/services/saf/amf/amfd/comptype.cc | 72 ++++++++++++++++++++++++++------- 1 files changed, 57 insertions(+), 15 deletions(-) Following commands causes amfd crash: immcfg -a saAmfCtDefQuiescingCompleteTimeout= safVersion=4.0.0,safCompType=OpenSafCompTypeAMFWDOG immcfg -a saAmfCtDefInstantiationLevel= safVersion=4.0.0,safCompType=OpenSafCompTypeAMFWDOG immcfg -a saAmfCtDefDisableRestart= safVersion=4.0.0,safCompType=OpenSafCompTypeAMFWDOG Patch adds check to prevent CCB modification to null value. diff --git a/osaf/services/saf/amf/amfd/comptype.cc b/osaf/services/saf/amf/amfd/comptype.cc --- a/osaf/services/saf/amf/amfd/comptype.cc +++ b/osaf/services/saf/amf/amfd/comptype.cc @@ -412,7 +412,7 @@ static void ccb_apply_modify_hdlr(const { const SaImmAttrModificationT_2 *attr_mod; int i; - const AVD_COMP_TYPE *comp_type; + AVD_COMP_TYPE *comp_type; SaNameT comp_type_name; TRACE_ENTER2("CCB ID %llu, '%s'", opdata->ccbId, opdata->objectName.value); @@ -473,13 +473,22 @@ static void ccb_apply_modify_hdlr(const param.attr_id = saAmfCtDefQuiescingCompleteTimeout_ID; avd_snd_op_req_msg(avd_cb, *it, ¶m); } else if (!strcmp(attribute->attrName, "saAmfCtDefInstantiationLevel")) { - SaUint32T *param_val = (SaUint32T *)attribute->attrValues[0]; - TRACE("saAmfCtDefInstantiationLevel to '%u' for compType '%s' on node '%s'", *param_val, + SaUint32T param_val; + SaUint32T old_value = comp_type->saAmfCtDefInstantiationLevel; + if ((attr_mod->modType == SA_IMM_ATTR_VALUES_DELETE) || + (attribute->attrValues == nullptr)) { + param_val = 0; //Default value as per Section 8.13.1 (B0401) + } else { + param_val = *(SaUint32T *)attribute->attrValues[0]; + } + TRACE("saAmfCtDefInstantiationLevel to '%u' for compType '%s' on +node '%s'", param_val, opdata->objectName.value, (*it)->name.value); - param.value_len = sizeof(*param_val); - memcpy(param.value, param_val, param.value_len); + param.value_len = sizeof(param_val); + memcpy(param.value, ¶m_val, param.value_len); param.attr_id = saAmfCtDefInstantiationLevel_ID; - avd_snd_op_req_msg(avd_cb, *it, ¶m); + comp_type->saAmfCtDefInstantiationLevel = param_val; + if (old_value != param_val) + avd_snd_op_req_msg(avd_cb, *it, ¶m); } else if (!strcmp(attribute->attrName, "saAmfCtDefRecoveryOnError")) { SaAmfRecommendedRecoveryT *param_val = (SaAmfRecommendedRecoveryT *)attribute->attrValues[0]; TRACE("saAmfCtDefRecoveryOnError to '%u' for compType '%s' on node '%s'", *param_val, @@ -489,13 +498,22 @@ static void ccb_apply_modify_hdlr(const param.attr_id = saAmfCtDefRecoveryOnError_ID; avd_snd_op_req_msg(avd_cb, *it, ¶m); } else if (!strcmp(attribute->attrName, "saAmfCtDefDisableRestart")) { - SaBoolT *param_val = (SaBoolT *)attribute->attrValues[0]; - TRACE("saAmfCtDefDisableRestart to '%u' for compType '%s' on node '%s'", *param_val, + SaBoolT param_val; + SaUint32T old_value = comp_type->saAmfCtDefDisableRestart; + if ((attr_mod->modType == SA_IMM_ATTR_VALUES_DELETE) || + (attribute->attrValues == nullptr)) { + param_val = static_cast<SaBoolT>(0); //Default value as per Section 8.13.1 (B0401) + } else { + param_val = *(SaBoolT *)attribute->attrValues[0]; + } + TRACE("saAmfCtDefDisableRestart to '%u' for compType '%s' on node +'%s'", param_val, opdata->objectName.value, (*it)->name.value); - param.value_len = sizeof(*param_val); - memcpy(param.value, param_val, param.value_len); + param.value_len = sizeof(param_val); + memcpy(param.value, ¶m_val, param.value_len); param.attr_id = saAmfCtDefDisableRestart_ID; - avd_snd_op_req_msg(avd_cb, *it, ¶m); + comp_type->saAmfCtDefDisableRestart = param_val; + if (old_value != param_val) + avd_snd_op_req_msg(avd_cb, *it, ¶m); } else LOG_WA("Unexpected attribute name: %s", attribute->attrName); } @@ -538,12 +556,21 @@ static SaAisErrorT ccb_completed_modify_ const SaImmAttrModificationT_2 *mod; int i = 0; const char *dn = (char*)opdata->objectName.value; + bool value_is_deleted = false; while ((mod = opdata->param.modify.attrMods[i++]) != nullptr) { + + if ((mod->modType == SA_IMM_ATTR_VALUES_DELETE) || (mod->modAttr.attrValues == nullptr)) { + /* Attribute value is deleted, revert to default value if applicable*/ + value_is_deleted = true; + } else { + /* Attribute value is modified */ + value_is_deleted = false; + } + if (strcmp(mod->modAttr.attrName, "saAmfCtDefClcCliTimeout") == 0) { // if it exist it cannot be removed, just changed - if ((mod->modType == SA_IMM_ATTR_VALUES_DELETE) || - (mod->modAttr.attrValues == nullptr)) { + if (value_is_deleted == true) { report_ccb_validation_error(opdata, "Value deletion for '%s' is not supported", mod->modAttr.attrName); rc = SA_AIS_ERR_BAD_OPERATION; @@ -559,8 +586,7 @@ static SaAisErrorT ccb_completed_modify_ } } else if (strcmp(mod->modAttr.attrName, "saAmfCtDefCallbackTimeout") == 0) { // if it exist it cannot be removed, just changed - if ((mod->modType == SA_IMM_ATTR_VALUES_DELETE) || - (mod->modAttr.attrValues == nullptr)) { + if (value_is_deleted == true) { report_ccb_validation_error(opdata, "Value deletion for '%s' is not supported", mod->modAttr.attrName); rc = SA_AIS_ERR_BAD_OPERATION; @@ -587,6 +613,12 @@ static SaAisErrorT ccb_completed_modify_ } else if (strcmp(mod->modAttr.attrName, "osafAmfCtDefHcCmdArgv") == 0) { ; // Allow modification, no validation can be done } else if (strcmp(mod->modAttr.attrName, "saAmfCtDefQuiescingCompleteTimeout") == 0) { + if (value_is_deleted == true) { + report_ccb_validation_error(opdata, + "Value deletion for '%s' is not supported", mod->modAttr.attrName); + rc = SA_AIS_ERR_BAD_OPERATION; + goto done; + } SaTimeT value = *((SaTimeT *)mod->modAttr.attrValues[0]); if (value < 100 * SA_TIME_ONE_MILLISECOND) { report_ccb_validation_error(opdata, @@ -595,6 +627,8 @@ static SaAisErrorT ccb_completed_modify_ goto done; } } else if (!strcmp(mod->modAttr.attrName, "saAmfCtDefInstantiationLevel")) { + if (value_is_deleted == true) + continue; uint32_t num_inst = *((SaUint32T *)mod->modAttr.attrValues[0]); if (num_inst == 0) { report_ccb_validation_error(opdata, "Modification of saAmfCtDefInstantiationLevel Fail," @@ -603,6 +637,12 @@ static SaAisErrorT ccb_completed_modify_ goto done; } } else if (strcmp(mod->modAttr.attrName, "saAmfCtDefRecoveryOnError") == 0) { + if (value_is_deleted == true) { + report_ccb_validation_error(opdata, + "Value deletion for '%s' is not supported", mod->modAttr.attrName); + rc = SA_AIS_ERR_BAD_OPERATION; + goto done; + } uint32_t value = *((SaUint32T *)mod->modAttr.attrValues[0]); if ((value < SA_AMF_COMPONENT_RESTART) || (value > SA_AMF_NODE_FAILFAST)) { report_ccb_validation_error(opdata, @@ -611,6 +651,8 @@ static SaAisErrorT ccb_completed_modify_ goto done; } } else if (strcmp(mod->modAttr.attrName, "saAmfCtDefDisableRestart") == 0) { + if (value_is_deleted == true) + continue; uint32_t value = *((SaUint32T *)mod->modAttr.attrValues[0]); if (value > SA_TRUE) { report_ccb_validation_error(opdata, ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
