Hi Mahesh, Could you please help to push the patch? Thanks.
Best regards, Nhat Pham -----Original Message----- From: A V Mahesh [mailto:mahesh.va...@oracle.com] Sent: Wednesday, May 25, 2016 1:02 PM To: Nhat Pham <nhat.p...@dektech.com.au>; anders.wid...@ericsson.com Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 1 of 1] cpnd: To erase element from section_db if inserting the element into local_section_db fails [#1843] V2 ACK -Please apply patch in both opensaf-5.0.x & opensaf-4.7.x -Sanity testing done on both opensaf-5.0.x & opensaf-5.0.x -Bug specific testing is rollback case on error and complex to reproduce. -AVM On 5/24/2016 8:58 AM, Nhat Pham wrote: > Hi Mahesh, > > I could not reproduce the fault. I just found the fault based on > backtrace and syslog. > > Best regards, > Nhat Pham > > -----Original Message----- > From: A V Mahesh [mailto:mahesh.va...@oracle.com] > Sent: Tuesday, May 24, 2016 10:13 AM > To: Nhat Pham <nhat.p...@dektech.com.au>; anders.wid...@ericsson.com > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [PATCH 1 of 1] cpnd: To erase element from section_db if > inserting the element into local_section_db fails [#1843] V2 > > Hi Nhat Pham, > > Thanks for the updated patch. > By the way, do you have any steps to reproduce ( in which scenario > this issue occurred ) I know that this is a rollback case and little > bit complex to reproduce , if you have any trick to reproduce please > provide . > > > -AVM > > On 5/24/2016 8:01 AM, Nhat Pham wrote: >> osaf/services/saf/cpsv/cpnd/cpnd_db.c | 31 > ++++++++++++++++++------------- >> osaf/services/saf/cpsv/cpnd/cpnd_res.c | 11 ++++++++++- >> osaf/services/saf/cpsv/cpnd/cpnd_sec.cc | 7 ++++++- >> 3 files changed, 34 insertions(+), 15 deletions(-) >> >> >> Problem: >> -------- >> There are 2 steps when a section is added in to the database >> (function > cpnd_ckpt_sec_add_db()): >> 1. The section information is inserted into section_db map 2. The >> section information is inserted into local_section_db map >> >> In case the step 2. fails, the cpnd_ckpt_sec_add_db() returns a fault >> code immediately without erasing the section information inserted in >> step 1. This leads a core dump triggered when the checkpoint replica >> is > deleted because there is an invalid element in the section_db map. >> Solution: >> --------- >> The solution is to erase the section information in step 1 in case >> the > step 2 fails. >> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_db.c >> b/osaf/services/saf/cpsv/cpnd/cpnd_db.c >> --- a/osaf/services/saf/cpsv/cpnd/cpnd_db.c >> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_db.c >> @@ -420,7 +420,7 @@ CPND_CKPT_SECTION_INFO *cpnd_ckpt_sec_ad >> pSecPtr->sec_id.id = > m_MMGR_ALLOC_CPND_DEFAULT(pSecPtr->sec_id.idLen); >> if (pSecPtr->sec_id.id == NULL) { >> LOG_ER("cpnd sect memory allocation failed"); >> - return NULL; >> + goto sec_id_allocate_fails; >> } >> memset(pSecPtr->sec_id.id, '\0', pSecPtr->sec_id.idLen); >> >> @@ -445,10 +445,7 @@ CPND_CKPT_SECTION_INFO *cpnd_ckpt_sec_ad >> rc = cpnd_ckpt_sec_add_db(&cp_node->replica_info, pSecPtr); >> if (rc == NCSCC_RC_FAILURE) { >> LOG_ER("unable to add section to database"); >> - m_MMGR_FREE_CPND_CPND_CKPT_SECTION_INFO(pSecPtr); >> - pSecPtr = NULL; >> - TRACE_LEAVE(); >> - return pSecPtr; >> + goto section_add_fails; >> } >> >> cp_node->replica_info.n_secs++; >> @@ -456,23 +453,31 @@ CPND_CKPT_SECTION_INFO *cpnd_ckpt_sec_ad >> /* UPDATE THE SECTION HEADER */ >> rc = cpnd_sec_hdr_update(pSecPtr, cp_node); >> if (rc == NCSCC_RC_FAILURE) { >> - TRACE_4("cpnd sect hdr update failed"); >> - cpnd_ckpt_sec_del(cp_node, id); >> - m_MMGR_FREE_CPND_CPND_CKPT_SECTION_INFO(pSecPtr); >> - pSecPtr = NULL; >> + LOG_NO("cpnd sect hdr update failed"); >> + goto section_hdr_update_fails; >> } >> /* UPDATE THE CHECKPOINT HEADER */ >> rc = cpnd_ckpt_hdr_update(cp_node); >> if (rc == NCSCC_RC_FAILURE) { >> - TRACE_4("cpnd ckpt hdr update failed"); >> - cpnd_ckpt_sec_del(cp_node, id); >> - m_MMGR_FREE_CPND_CPND_CKPT_SECTION_INFO(pSecPtr); >> - pSecPtr = NULL; >> + LOG_NO("cpnd ckpt hdr update failed"); >> + goto ckpt_hdr_update_fails; >> } >> >> TRACE_LEAVE(); >> return pSecPtr; >> >> + section_hdr_update_fails: >> + ckpt_hdr_update_fails: >> + cpnd_ckpt_sec_del(cp_node, id); >> + >> + section_add_fails: >> + if (pSecPtr->sec_id.id != NULL) >> + m_MMGR_FREE_CPND_DEFAULT(pSecPtr->sec_id.id); >> + >> + sec_id_allocate_fails: >> + m_MMGR_FREE_CPND_CPND_CKPT_SECTION_INFO(pSecPtr); >> + TRACE_LEAVE(); >> + return NULL; >> } >> >> >> /******************************************************************** >> * >> ******* diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_res.c >> b/osaf/services/saf/cpsv/cpnd/cpnd_res.c >> --- a/osaf/services/saf/cpsv/cpnd/cpnd_res.c >> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_res.c >> @@ -245,6 +245,7 @@ uint32_t cpnd_ckpt_replica_create_res(NC >> pSecPtr->sec_id.id = > m_MMGR_ALLOC_CPND_DEFAULT(pSecPtr->sec_id.idLen); >> if (pSecPtr->sec_id.id == NULL) { >> TRACE_4("cpnd default allocation failed for > sec_id length"); >> + > m_MMGR_FREE_CPND_CPND_CKPT_SECTION_INFO(pSecPtr); >> rc = NCSCC_RC_FAILURE; >> goto end; >> } >> @@ -256,7 +257,15 @@ uint32_t cpnd_ckpt_replica_create_res(NC >> pSecPtr->exp_tmr = sect_hdr.exp_tmr; >> pSecPtr->lastUpdate = sect_hdr.lastUpdate; >> >> - cpnd_res_ckpt_sec_add(pSecPtr, *cp_node); >> + rc = cpnd_res_ckpt_sec_add(pSecPtr, *cp_node); >> + if (rc != NCSCC_RC_SUCCESS) { >> + LOG_NO("cpnd restart create replica - add section > fails"); >> + if (pSecPtr->sec_id.id != NULL) >> + > m_MMGR_FREE_CPND_DEFAULT(pSecPtr->sec_id.id); \ >> + m_MMGR_FREE_CPND_CPND_CKPT_SECTION_INFO(pSecPtr); >> + goto end; >> + } >> + >> memset(§_hdr, '\0', sizeof(CPSV_SECT_HDR)); >> >> (*cp_node)->replica_info.mem_used += pSecPtr->sec_size; diff > --git >> a/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc >> b/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc >> --- a/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc >> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_sec.cc >> @@ -255,6 +255,7 @@ cpnd_ckpt_sec_add_db(CPND_CKPT_REPLICA_I >> if (!p.second) { >> LOG_ER("unable to add section info to map"); >> rc = NCSCC_RC_FAILURE; >> + return rc; >> } >> } >> else { >> @@ -270,8 +271,12 @@ cpnd_ckpt_sec_add_db(CPND_CKPT_REPLICA_I >> std::make_pair(sectionInfo->lcl_sec_id, sectionInfo))); >> >> if (!p.second) { >> - LOG_ER("unable to add section info to local section id map"); >> + LOG_ER("unable to add section info to local section id map - >> + the id > %d already existed", >> + sectionInfo->lcl_sec_id); >> rc = NCSCC_RC_FAILURE; >> + >> + /* Erase the element was inserted into section_db */ >> + map->erase(§ionInfo->sec_id); >> } >> } >> else { > ------------------------------------------------------------------------------ Mobile security can be enabling, not merely restricting. Employees who bring their own devices (BYOD) to work are irked by the imposition of MDM restrictions. Mobile Device Manager Plus allows you to control only the apps on BYO-devices by containerizing them, leaving personal data untouched! https://ad.doubleclick.net/ddm/clk/304595813;131938128;j _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
