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
