Ack, code review only, I assume you had run some regression on it. Thanks -Nagu
> -----Original Message----- > From: Hoa Le [mailto:[email protected]] > Sent: 25 April 2017 12:21 > To: [email protected]; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected] > Cc: [email protected]; Hoa Le > Subject: [PATCH 1/1] amfnd: Fix illegal memory access in > avnd_comptype_delete [#2424] > > Problem: > - There are some unsafe memory accesses which may cause segfault in > avnd_comptype_delete() function. > > Fix: > - Check if a pointer is valid before accessing it. > - Minor update in avnd_comptype_create() function to avoid unnecessary > function call to avnd_comptype_delete(). > --- > src/amf/amfnd/compdb.cc | 95 +++++++++++++++++++++++++++------------- > --------- > 1 file changed, 53 insertions(+), 42 deletions(-) > > diff --git a/src/amf/amfnd/compdb.cc b/src/amf/amfnd/compdb.cc > index cf6e3ff..decf3ab 100644 > --- a/src/amf/amfnd/compdb.cc > +++ b/src/amf/amfnd/compdb.cc > @@ -766,48 +766,67 @@ static void > avnd_comptype_delete(amf_comp_type_t *compt) { > > delete[] compt->saAmfCtRelPathInstantiateCmd; > > - /* Free saAmfCtDefInstantiateCmdArgv[i] before freeing > - * saAmfCtDefInstantiateCmdArgv */ > - arg_counter = 0; > - while ((argv = compt->saAmfCtDefInstantiateCmdArgv[arg_counter++]) != > nullptr) > - delete[] argv; > - delete[] compt->saAmfCtDefInstantiateCmdArgv; > + if (compt->saAmfCtDefInstantiateCmdArgv != nullptr) { > + /* Free saAmfCtDefInstantiateCmdArgv[i] before freeing > + * saAmfCtDefInstantiateCmdArgv */ > + arg_counter = 0; > + while ((argv = compt->saAmfCtDefInstantiateCmdArgv[arg_counter++]) > != > + nullptr) > + delete[] argv; > + delete[] compt->saAmfCtDefInstantiateCmdArgv; > + } > > delete[] compt->saAmfCtRelPathTerminateCmd; > > - /* Free saAmfCtDefTerminateCmdArgv[i] before freeing > - * saAmfCtDefTerminateCmdArgv */ > - arg_counter = 0; > - while ((argv = compt->saAmfCtDefTerminateCmdArgv[arg_counter++]) != > nullptr) > - delete[] argv; > - delete[] compt->saAmfCtDefTerminateCmdArgv; > + if (compt->saAmfCtDefTerminateCmdArgv != nullptr) { > + /* Free saAmfCtDefTerminateCmdArgv[i] before freeing > + * saAmfCtDefTerminateCmdArgv */ > + arg_counter = 0; > + while ((argv = compt->saAmfCtDefTerminateCmdArgv[arg_counter++]) != > nullptr) > + delete[] argv; > + delete[] compt->saAmfCtDefTerminateCmdArgv; > + } > > delete[] compt->saAmfCtRelPathCleanupCmd; > - /* Free saAmfCtDefCleanupCmdArgv[i] before freeing > saAmfCtDefCleanupCmdArgv */ > - arg_counter = 0; > - while ((argv = compt->saAmfCtDefCleanupCmdArgv[arg_counter++]) != > nullptr) > - delete[] argv; > - delete[] compt->saAmfCtDefCleanupCmdArgv; > + > + if (compt->saAmfCtDefCleanupCmdArgv != nullptr) { > + /* Free saAmfCtDefCleanupCmdArgv[i] before freeing > + * saAmfCtDefCleanupCmdArgv */ > + arg_counter = 0; > + while ((argv = compt->saAmfCtDefCleanupCmdArgv[arg_counter++]) != > nullptr) > + delete[] argv; > + delete[] compt->saAmfCtDefCleanupCmdArgv; > + } > > delete[] compt->saAmfCtRelPathAmStartCmd; > - /* Free saAmfCtDefAmStartCmdArgv[i] before freeing > saAmfCtDefAmStartCmdArgv */ > - arg_counter = 0; > - while ((argv = compt->saAmfCtDefAmStartCmdArgv[arg_counter++]) != > nullptr) > - delete[] argv; > - delete[] compt->saAmfCtDefAmStartCmdArgv; > + > + if (compt->saAmfCtDefAmStartCmdArgv != nullptr) { > + /* Free saAmfCtDefAmStartCmdArgv[i] before freeing > + * saAmfCtDefAmStartCmdArgv */ > + arg_counter = 0; > + while ((argv = compt->saAmfCtDefAmStartCmdArgv[arg_counter++]) != > nullptr) > + delete[] argv; > + delete[] compt->saAmfCtDefAmStartCmdArgv; > + } > > delete[] compt->saAmfCtRelPathAmStopCmd; > - /* Free saAmfCtDefAmStopCmdArgv[i] before freeing > saAmfCtDefAmStopCmdArgv */ > - arg_counter = 0; > - while ((argv = compt->saAmfCtDefAmStopCmdArgv[arg_counter++]) != > nullptr) > - delete[] argv; > - delete[] compt->saAmfCtDefAmStopCmdArgv; > + > + if (compt->saAmfCtDefAmStopCmdArgv != nullptr) { > + /* Free saAmfCtDefAmStopCmdArgv[i] before freeing > saAmfCtDefAmStopCmdArgv */ > + arg_counter = 0; > + while ((argv = compt->saAmfCtDefAmStopCmdArgv[arg_counter++]) != > nullptr) > + delete[] argv; > + delete[] compt->saAmfCtDefAmStopCmdArgv; > + } > > delete[] compt->osafAmfCtRelPathHcCmd; > - arg_counter = 0; > - while ((argv = compt->osafAmfCtDefHcCmdArgv[arg_counter++]) != > nullptr) > - delete[] argv; > - delete[] compt->osafAmfCtDefHcCmdArgv; > + > + if (compt->osafAmfCtDefHcCmdArgv != nullptr) { > + arg_counter = 0; > + while ((argv = compt->osafAmfCtDefHcCmdArgv[arg_counter++]) != > nullptr) > + delete[] argv; > + delete[] compt->osafAmfCtDefHcCmdArgv; > + } > > delete compt; > > @@ -817,8 +836,7 @@ static void > avnd_comptype_delete(amf_comp_type_t *compt) { > static amf_comp_type_t *avnd_comptype_create(SaImmHandleT > immOmHandle, > const std::string &dn) { > SaImmAccessorHandleT accessorHandle; > - amf_comp_type_t *compt; > - int rc = -1; > + amf_comp_type_t *compt = nullptr; > unsigned int i; > unsigned int j; > const char *str; > @@ -827,8 +845,6 @@ static amf_comp_type_t > *avnd_comptype_create(SaImmHandleT immOmHandle, > > TRACE_ENTER2("'%s'", dn.c_str()); > > - compt = new amf_comp_type_t(); > - > (void)amf_saImmOmAccessorInitialize(immOmHandle, accessorHandle); > > if (amf_saImmOmAccessorGet_o2(immOmHandle, accessorHandle, dn, > nullptr, > @@ -838,6 +854,8 @@ static amf_comp_type_t > *avnd_comptype_create(SaImmHandleT immOmHandle, > goto done; > } > > + compt = new amf_comp_type_t(); > + > compt->name = dn; > > if > (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfCtCompCategory"), > @@ -993,14 +1011,7 @@ static amf_comp_type_t > *avnd_comptype_create(SaImmHandleT immOmHandle, > &compt->saAmfCtDefDisableRestart) != SA_AIS_OK) > compt->saAmfCtDefDisableRestart = false; > > - rc = 0; > - > done: > - if (rc != 0) { > - avnd_comptype_delete(compt); > - compt = nullptr; > - } > - > (void)immutil_saImmOmAccessorFinalize(accessorHandle); > > TRACE_LEAVE(); > -- > 2.9.3 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
