From e7f7c4274e6021ea8ee5d44e606a4f90f3153a76 Mon Sep 17 00:00:00 2001 From: Sukchan Lee Date: Thu, 10 Aug 2023 22:14:48 +0900 Subject: [PATCH] [SMF] Fix crash on double policy deletion (#2489) --- src/amf/context.h | 2 +- src/mme/mme-context.h | 6 +++--- src/smf/context.h | 4 +++- src/smf/gsm-sm.c | 4 ---- src/smf/npcf-handler.c | 20 +++++++++++++------- src/smf/nsmf-handler.c | 11 +++++++++-- src/smf/pfcp-sm.c | 38 ++++++++++++++++++++++---------------- 7 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/amf/context.h b/src/amf/context.h index 1f41ab8f3..c6ada8d22 100644 --- a/src/amf/context.h +++ b/src/amf/context.h @@ -761,7 +761,7 @@ amf_sess_t *amf_sess_add(amf_ue_t *amf_ue, uint8_t psi); do { \ ogs_sbi_object_t *sbi_object = NULL; \ ogs_assert(__sESS); \ - sbi_object = &sess->sbi; \ + sbi_object = &(__sESS)->sbi; \ ogs_assert(sbi_object); \ \ ogs_error("AMF_SESS_CLEAR"); \ diff --git a/src/mme/mme-context.h b/src/mme/mme-context.h index 3c0da394b..9ef19e92a 100644 --- a/src/mme/mme-context.h +++ b/src/mme/mme-context.h @@ -677,14 +677,14 @@ struct mme_ue_s { do { \ mme_ue_t *mme_ue = NULL; \ ogs_assert(__sESS); \ - mme_ue = __sESS->mme_ue; \ + mme_ue = (__sESS)->mme_ue; \ ogs_assert(mme_ue); \ ogs_info("Removed Session: UE IMSI:[%s] APN:[%s]", \ mme_ue->imsi_bcd, \ - sess->session ? sess->session->name : "Unknown"); \ + (__sESS)->session ? (__sESS)->session->name : "Unknown"); \ if (mme_sess_count(mme_ue) == 1) /* Last Session */ \ CLEAR_SESSION_CONTEXT(mme_ue); \ - mme_sess_remove(sess); \ + mme_sess_remove(__sESS); \ } while(0) #define ACTIVE_EPS_BEARERS_IS_AVAIABLE(__mME) \ diff --git a/src/smf/context.h b/src/smf/context.h index 37c48e716..3fff192e0 100644 --- a/src/smf/context.h +++ b/src/smf/context.h @@ -138,8 +138,10 @@ typedef struct smf_ue_s { do { \ smf_ue_t *smf_ue = NULL; \ ogs_assert(__sESS); \ - smf_ue = __sESS->smf_ue; \ + smf_ue = (__sESS)->smf_ue; \ ogs_assert(smf_ue); \ + smf_metrics_inst_by_slice_add(&(__sESS)->plmn_id, \ + &(__sESS)->s_nssai, SMF_METR_GAUGE_SM_SESSIONNBR, -1); \ if (SMF_UE_IS_LAST_SESSION(smf_ue)) \ smf_ue_remove(smf_ue); \ else \ diff --git a/src/smf/gsm-sm.c b/src/smf/gsm-sm.c index df1ce18cb..b84553f2a 100644 --- a/src/smf/gsm-sm.c +++ b/src/smf/gsm-sm.c @@ -1830,8 +1830,6 @@ void smf_gsm_state_session_will_release(ogs_fsm_t *s, smf_event_t *e) switch (e->h.id) { case OGS_FSM_ENTRY_SIG: - smf_metrics_inst_by_slice_add(&sess->plmn_id, &sess->s_nssai, - SMF_METR_GAUGE_SM_SESSIONNBR, -1); SMF_SESS_CLEAR(sess); break; @@ -1862,8 +1860,6 @@ void smf_gsm_state_exception(ogs_fsm_t *s, smf_event_t *e) switch (e->h.id) { case OGS_FSM_ENTRY_SIG: ogs_error("[%s:%d] State machine exception", smf_ue->supi, sess->psi); - smf_metrics_inst_by_slice_add(&sess->plmn_id, &sess->s_nssai, - SMF_METR_GAUGE_SM_SESSIONNBR, -1); SMF_SESS_CLEAR(sess); break; diff --git a/src/smf/npcf-handler.c b/src/smf/npcf-handler.c index 5c5f5ba88..3d2d3f46d 100644 --- a/src/smf/npcf-handler.c +++ b/src/smf/npcf-handler.c @@ -713,13 +713,19 @@ bool smf_npcf_smpolicycontrol_handle_terminate_notify( ogs_assert(true == ogs_sbi_send_http_status_no_content(stream)); - memset(¶m, 0, sizeof(param)); - r = smf_sbi_discover_and_send( - OGS_SBI_SERVICE_TYPE_NPCF_SMPOLICYCONTROL, NULL, - smf_npcf_smpolicycontrol_build_delete, - sess, NULL, OGS_PFCP_DELETE_TRIGGER_PCF_INITIATED, ¶m); - ogs_expect(r == OGS_OK); - ogs_assert(r != OGS_ERROR); + if (sess->policy_association_id) { + memset(¶m, 0, sizeof(param)); + r = smf_sbi_discover_and_send( + OGS_SBI_SERVICE_TYPE_NPCF_SMPOLICYCONTROL, NULL, + smf_npcf_smpolicycontrol_build_delete, + sess, NULL, OGS_PFCP_DELETE_TRIGGER_PCF_INITIATED, ¶m); + ogs_expect(r == OGS_OK); + ogs_assert(r != OGS_ERROR); + } else { + ogs_error("[%s:%d] No PolicyAssociationId. Forcibly remove SESSION", + smf_ue->supi, sess->psi); + SMF_SESS_CLEAR(sess); + } return true; } diff --git a/src/smf/nsmf-handler.c b/src/smf/nsmf-handler.c index 4533458dd..f248ec054 100644 --- a/src/smf/nsmf-handler.c +++ b/src/smf/nsmf-handler.c @@ -682,10 +682,12 @@ bool smf_nsmf_handle_update_sm_context( ogs_expect(r == OGS_OK); ogs_assert(r != OGS_ERROR); } else { - ogs_error("No PolicyAssociationId"); + ogs_error("[%s:%d] No PolicyAssociationId. Forcibly remove SESSION", + smf_ue->supi, sess->psi); smf_sbi_send_sm_context_update_error_log( stream, OGS_SBI_HTTP_STATUS_NOT_FOUND, "No PolicyAssociationId", NULL); + SMF_SESS_CLEAR(sess); } } else if (SmContextUpdateData->serving_nf_id) { ogs_debug("Old serving_nf_id: %s, new serving_nf_id: %s", @@ -717,12 +719,15 @@ bool smf_nsmf_handle_release_sm_context( { int r; smf_npcf_smpolicycontrol_param_t param; + smf_ue_t *smf_ue = NULL; OpenAPI_sm_context_release_data_t *SmContextReleaseData = NULL; ogs_assert(stream); ogs_assert(message); ogs_assert(sess); + smf_ue = sess->smf_ue; + ogs_assert(smf_ue); memset(¶m, 0, sizeof(param)); @@ -774,11 +779,13 @@ bool smf_nsmf_handle_release_sm_context( ogs_expect(r == OGS_OK); ogs_assert(r != OGS_ERROR); } else { - ogs_error("No PolicyAssociationId"); + ogs_error("[%s:%d] No PolicyAssociationId. Forcibly remove SESSION", + smf_ue->supi, sess->psi); ogs_assert(true == ogs_sbi_server_send_error( stream, OGS_SBI_HTTP_STATUS_NOT_FOUND, NULL, "No PolicyAssociationId", NULL)); + SMF_SESS_CLEAR(sess); } return true; diff --git a/src/smf/pfcp-sm.c b/src/smf/pfcp-sm.c index ab4f81100..b426f73a0 100644 --- a/src/smf/pfcp-sm.c +++ b/src/smf/pfcp-sm.c @@ -499,30 +499,36 @@ static void reselect_upf(ogs_pfcp_node_t *node) } ogs_list_for_each(&smf_self()->smf_ue_list, smf_ue) { - smf_sess_t *sess = NULL; - ogs_assert(smf_ue); + smf_sess_t *sess = NULL, *next_sess = NULL; - ogs_list_for_each(&smf_ue->sess_list, sess) { - ogs_assert(sess); + ogs_list_for_each_safe(&smf_ue->sess_list, next_sess, sess) { if (node == sess->pfcp_node) { if (sess->epc) { ogs_error("[%s:%s] EPC restoration is not implemented", smf_ue->imsi_bcd, sess->session.name); } else { - smf_npcf_smpolicycontrol_param_t param; + if (sess->policy_association_id) { + smf_npcf_smpolicycontrol_param_t param; - ogs_info("[%s:%d] SMF-initiated Deletion", - smf_ue->supi, sess->psi); - ogs_assert(sess->sm_context_ref); - memset(¶m, 0, sizeof(param)); - r = smf_sbi_discover_and_send( - OGS_SBI_SERVICE_TYPE_NPCF_SMPOLICYCONTROL, NULL, - smf_npcf_smpolicycontrol_build_delete, - sess, NULL, OGS_PFCP_DELETE_TRIGGER_SMF_INITIATED, - ¶m); - ogs_expect(r == OGS_OK); - ogs_assert(r != OGS_ERROR); + ogs_info("[%s:%d] SMF-initiated Deletion", + smf_ue->supi, sess->psi); + ogs_assert(sess->sm_context_ref); + memset(¶m, 0, sizeof(param)); + r = smf_sbi_discover_and_send( + OGS_SBI_SERVICE_TYPE_NPCF_SMPOLICYCONTROL, NULL, + smf_npcf_smpolicycontrol_build_delete, + sess, NULL, + OGS_PFCP_DELETE_TRIGGER_SMF_INITIATED, + ¶m); + ogs_expect(r == OGS_OK); + ogs_assert(r != OGS_ERROR); + } else { + ogs_error("[%s:%d] No PolicyAssociationId. " + "Forcibly remove SESSION", + smf_ue->supi, sess->psi); + SMF_SESS_CLEAR(sess); + } } } }