[devel] [PATCH 1/1] amfnd: Fix illegal memory access in avnd_comptype_delete [#2424]

[Hoa Le]

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 | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/amf/amfnd/compdb.cc b/src/amf/amfnd/compdb.cc
index cf6e3ff..d0b9f38 100644
--- a/src/amf/amfnd/compdb.cc
+++ b/src/amf/amfnd/compdb.cc
@@ -757,15 +757,15 @@ static void avnd_comptype_delete(amf_comp_type_t *compt) {
 
   TRACE("'%s'", compt->name.c_str());
   /* Free saAmfCtDefCmdEnv[i] before freeing saAmfCtDefCmdEnv */
-  if (compt->saAmfCtDefCmdEnv != nullptr) {
-    arg_counter = 0;
-    while ((argv = compt->saAmfCtDefCmdEnv[arg_counter++]) != nullptr)
-      delete[] argv;
-    delete[] compt->saAmfCtDefCmdEnv;
-  }
+  osafassert(compt->saAmfCtDefCmdEnv != nullptr);
+  arg_counter = 0;
+  while ((argv = compt->saAmfCtDefCmdEnv[arg_counter++]) != nullptr)
+    delete[] argv;
+  delete[] compt->saAmfCtDefCmdEnv;
 
   delete[] compt->saAmfCtRelPathInstantiateCmd;
 
+  osafassert(compt->saAmfCtDefInstantiateCmdArgv != nullptr);
   /* Free saAmfCtDefInstantiateCmdArgv[i] before freeing
    * saAmfCtDefInstantiateCmdArgv */
   arg_counter = 0;
@@ -775,6 +775,7 @@ static void avnd_comptype_delete(amf_comp_type_t *compt) {
 
   delete[] compt->saAmfCtRelPathTerminateCmd;
 
+  osafassert(compt->saAmfCtDefTerminateCmdArgv != nullptr);
   /* Free saAmfCtDefTerminateCmdArgv[i] before freeing
    * saAmfCtDefTerminateCmdArgv */
   arg_counter = 0;
@@ -783,6 +784,8 @@ static void avnd_comptype_delete(amf_comp_type_t *compt) {
   delete[] compt->saAmfCtDefTerminateCmdArgv;
 
   delete[] compt->saAmfCtRelPathCleanupCmd;
+
+  osafassert(compt->saAmfCtDefCleanupCmdArgv != nullptr);
   /* Free saAmfCtDefCleanupCmdArgv[i] before freeing saAmfCtDefCleanupCmdArgv 
*/
   arg_counter = 0;
   while ((argv = compt->saAmfCtDefCleanupCmdArgv[arg_counter++]) != nullptr)
@@ -790,6 +793,8 @@ static void avnd_comptype_delete(amf_comp_type_t *compt) {
   delete[] compt->saAmfCtDefCleanupCmdArgv;
 
   delete[] compt->saAmfCtRelPathAmStartCmd;
+
+  osafassert(compt->saAmfCtDefAmStartCmdArgv != nullptr);
   /* Free saAmfCtDefAmStartCmdArgv[i] before freeing saAmfCtDefAmStartCmdArgv 
*/
   arg_counter = 0;
   while ((argv = compt->saAmfCtDefAmStartCmdArgv[arg_counter++]) != nullptr)
@@ -797,6 +802,8 @@ static void avnd_comptype_delete(amf_comp_type_t *compt) {
   delete[] compt->saAmfCtDefAmStartCmdArgv;
 
   delete[] compt->saAmfCtRelPathAmStopCmd;
+
+  osafassert(compt->saAmfCtDefAmStopCmdArgv != nullptr);
   /* Free saAmfCtDefAmStopCmdArgv[i] before freeing saAmfCtDefAmStopCmdArgv */
   arg_counter = 0;
   while ((argv = compt->saAmfCtDefAmStopCmdArgv[arg_counter++]) != nullptr)
@@ -804,6 +811,8 @@ static void avnd_comptype_delete(amf_comp_type_t *compt) {
   delete[] compt->saAmfCtDefAmStopCmdArgv;
 
   delete[] compt->osafAmfCtRelPathHcCmd;
+
+  osafassert(compt->osafAmfCtDefHcCmdArgv != nullptr);
   arg_counter = 0;
   while ((argv = compt->osafAmfCtDefHcCmdArgv[arg_counter++]) != nullptr)
     delete[] argv;
@@ -817,8 +826,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 +835,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 +844,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 +1001,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.7.4


------------------------------------------------------------------------------
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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel