Ack (review only)
On 25/4/17, 4:51 pm, "Hoa Le" <[email protected]> wrote:
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
