Hi Zoran, Reviewed and tested the patch. Ack from me.
BR, Hung Nguyen - DEK Technologies -------------------------------------------------------------------------------- From: Zoran Milinkovic [email protected] Sent: Monday, February 13, 2017 7:40PM To: Neelakanta Reddy [email protected] Cc: Opensaf-devel [email protected] Subject: [devel] [PATCH 1 of 1] imm: fix PBE coredump for double freeing memory [#2304] src/imm/common/immpbe_dump.cc | 29 +++++++++++++++++++---------- src/imm/immpbed/immpbe_daemon.cc | 2 ++ 2 files changed, 21 insertions(+), 10 deletions(-) A static string variable sPbeFileName is double freed when exit call is called from 2 threads. The patch dynamicly allocate the variable on pbeRepositoryInit call and free the variable on pbeRepositoryClose call. diff --git a/src/imm/common/immpbe_dump.cc b/src/imm/common/immpbe_dump.cc --- a/src/imm/common/immpbe_dump.cc +++ b/src/imm/common/immpbe_dump.cc @@ -72,7 +72,7 @@ void pbeClosePrepareTrans() #include <sqlite3.h> #define STRINT_BSZ 32 -static std::string sPbeFileName; +static std::string *sPbeFileName; #define SQL_STMT_SIZE 31 @@ -598,6 +598,10 @@ void* pbeRepositoryInit(const char* file }; TRACE_ENTER(); + if(!sPbeFileName) { + sPbeFileName = new std::string; + } + if(!create) {goto re_attach;} /* Create a fresh Pbe-repository by dumping current imm contents to (local or global) @@ -707,7 +711,7 @@ void* pbeRepositoryInit(const char* file prepareSqlStatements(dbHandle); - sPbeFileName = std::string(filePath); + *sPbeFileName = std::string(filePath); if (localTmpDir) free(localTmpDir); TRACE_LEAVE(); return (void *) dbHandle; @@ -799,7 +803,7 @@ void* pbeRepositoryInit(const char* file TRACE("Successfully executed %s", sql_tr[0]); - sPbeFileName = std::string(filePath); /* Avoid apend to presumed empty string */ + *sPbeFileName = std::string(filePath); /* Avoid apend to presumed empty string */ prepareSqlStatements(dbHandle); @@ -826,6 +830,11 @@ void pbeRepositoryClose(void* dbHandle) { finalizeSqlStatements(); sqlite3_close((sqlite3 *) dbHandle); + + if(sPbeFileName) { + delete sPbeFileName; + sPbeFileName = NULL; + } } void pbeCleanTmpFiles(std::string localTmpFilename) @@ -1537,7 +1546,7 @@ void objectModifyDiscardAllValuesOfAttrT TRACE_LEAVE(); sqlite3_close((sqlite3 *) dbHandle); if(badfile) { - discardPbeFile(sPbeFileName); + discardPbeFile(*sPbeFileName); } LOG_ER("Exiting (line:%u)", __LINE__); exit(1); @@ -1783,7 +1792,7 @@ void objectModifyDiscardMatchingValuesOf TRACE_LEAVE(); sqlite3_close((sqlite3 *) dbHandle); if(badfile) { - discardPbeFile(sPbeFileName); + discardPbeFile(*sPbeFileName); } LOG_ER("Exiting (line:%u)", __LINE__); @@ -1974,7 +1983,7 @@ void objectModifyAddValuesOfAttrToPBE(vo bailout: sqlite3_close((sqlite3 *) dbHandle); if(badfile) { - discardPbeFile(sPbeFileName); + discardPbeFile(*sPbeFileName); } LOG_ER("Exiting (line:%u)", __LINE__); exit(1); @@ -2301,7 +2310,7 @@ void objectDeleteToPBE(std::string objec bailout: sqlite3_close((sqlite3 *) dbHandle); if(badfile) { - discardPbeFile(sPbeFileName); + discardPbeFile(*sPbeFileName); } LOG_ER("Exiting (line:%u)", __LINE__); exit(1); @@ -2584,7 +2593,7 @@ int verifyPbeState(SaImmHandleT immHandl sqlite3_free_table(result); sqlite3_close(dbHandle); if(badfile) { - discardPbeFile(sPbeFileName); + discardPbeFile(*sPbeFileName); } LOG_WA("verifyPbeState failed!"); return 0; @@ -3103,7 +3112,7 @@ SaAisErrorT getCcbOutcomeFromPbe(void* d bailout: sqlite3_close(dbHandle); if(badfile) { - discardPbeFile(sPbeFileName); + discardPbeFile(*sPbeFileName); } LOG_ER("Exiting (line:%u)", __LINE__); exit(1); @@ -3112,7 +3121,7 @@ SaAisErrorT getCcbOutcomeFromPbe(void* d void fsyncPbeJournalFile() { int fd=(-1); - std::string globalJournalFilename(sPbeFileName); + std::string globalJournalFilename(*sPbeFileName); globalJournalFilename.append("-journal"); fd = open(globalJournalFilename.c_str(), O_RDWR); if(fd != (-1)) { diff --git a/src/imm/immpbed/immpbe_daemon.cc b/src/imm/immpbed/immpbe_daemon.cc --- a/src/imm/immpbed/immpbe_daemon.cc +++ b/src/imm/immpbed/immpbe_daemon.cc @@ -2287,6 +2287,8 @@ void pbeDaemon(SaImmHandleT immHandle, v continue; LOG_ER("poll failed - %s", strerror(errno)); + pbeRepositoryClose(sDbHandle); + sDbHandle = NULL; break; } ------------------------------------------------------------------------------ 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 ------------------------------------------------------------------------------ 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
