[mme] turn various asserts into checks with error-handling (#69)

README.md

@@ -34,4 +34,7 @@ Wherever possible, we have changed PFCP messages/operations from an action-based
 When a UPS re-associates itself with the CPS after a period of disconnectivity, their respective states may have diverged substantially. We handle this using a reassociation process wherein (1) the CPS sends a Session-Set-Delete message, effectively instructing the UPS to delete any/all current sessions, and then (2) the CPS re-sends a Session Establishment message for each active session. This creates a bit more chatter over the wire than you might expect, but works incredibly well in our context.
 
 ## Fine-Grained Timers
-Stock open5gs has one configurable parameter (`message.duration`) which sets the timeout value for *all* messages sent or received by the program in question. We turned this parameter into several different ones pertaining to each message protocol to allow us to have finer-grained control over these timers, which is highly recommended by the 3GPP. Specifically, `message.duration` has been replaced by `message.sbi_duration`, `message.gtp_duration`, `message.pfcp_duration` and `message.diameter_timeout`.
+Stock open5gs has one configurable parameter (`message.duration`) which sets the timeout value for *all* messages sent or received by the program in question. We turned this parameter into several different ones pertaining to each message protocol to allow us to have finer-grained control over these timers, which is highly recommended by the 3GPP. Specifically, `message.duration` has been replaced by `message.sbi_duration`, `message.gtp_duration`, `message.pfcp_duration` and `message.diameter_timeout`.
+
+## Stability Fixes
+We have caught and fixed a large number of small bugs that threaten stability, generally asserts() that are not always true. Our team is firmly committed to contributing all such fixes to the main open5gs project, and these commits are either already upstreamed or will be soon.

src/mme/mme-s11-handler.c

@@ -112,16 +112,6 @@ void mme_s11_handle_create_session_response(
     ogs_assert(sess);
     mme_ue = sess->mme_ue;
     ogs_assert(mme_ue);
-    source_ue = sgw_ue_cycle(mme_ue->sgw_ue);
-    ogs_assert(source_ue);
-
-    if (create_action == OGS_GTP_CREATE_IN_PATH_SWITCH_REQUEST) {
-        target_ue = sgw_ue_cycle(source_ue->target_ue);
-        ogs_assert(target_ue);
-    } else {
-        target_ue = source_ue;
-        ogs_assert(target_ue);
-    }
 
     rv = ogs_gtp_xact_commit(xact);
     ogs_expect_or_return(rv == OGS_OK);
@@ -136,6 +126,23 @@ void mme_s11_handle_create_session_response(
         cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
     }
 
+    source_ue = sgw_ue_cycle(mme_ue->sgw_ue);
+    if (!source_ue) {
+        ogs_error("Cannot find source_ue context");
+        cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;        
+    }
+
+    if (create_action == OGS_GTP_CREATE_IN_PATH_SWITCH_REQUEST) {
+        // if source_ue == null we'll catch below
+        if (source_ue) {
+            target_ue = sgw_ue_cycle(source_ue->target_ue);
+            ogs_assert(target_ue);            
+        }
+    } else {
+        target_ue = source_ue;
+        // if source_ue == null we'll catch below
+    }
+
     if (cause_value != OGS_GTP2_CAUSE_REQUEST_ACCEPTED) {
         if (create_action == OGS_GTP_CREATE_IN_ATTACH_REQUEST) {
             ogs_error("[%s] Attach reject", mme_ue->imsi_bcd);
@@ -437,8 +444,6 @@ void mme_s11_handle_modify_bearer_response(
     modify_action = xact->modify_action;
     mme_ue = xact->data;
     ogs_assert(mme_ue);
-    sgw_ue = sgw_ue_cycle(mme_ue->sgw_ue);
-    ogs_assert(sgw_ue);
 
     rv = ogs_gtp_xact_commit(xact);
     ogs_expect_or_return(rv == OGS_OK);
@@ -453,6 +458,12 @@ void mme_s11_handle_modify_bearer_response(
         cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
     }
 
+    sgw_ue = sgw_ue_cycle(mme_ue->sgw_ue);
+    if (!sgw_ue) {
+        ogs_error("Cannot find sgw_ue context");
+        cause_value = OGS_GTP2_CAUSE_CONTEXT_NOT_FOUND;
+    }
+
     if (cause_value != OGS_GTP2_CAUSE_REQUEST_ACCEPTED) {
         mme_send_delete_session_or_mme_ue_context_release(mme_ue);
         return;
@@ -1116,8 +1127,6 @@ void mme_s11_handle_release_access_bearers_response(
     ogs_assert(action);
     mme_ue = xact->data;
     ogs_assert(mme_ue);
-    sgw_ue = sgw_ue_cycle(mme_ue->sgw_ue);
-    ogs_assert(sgw_ue);
 
     rv = ogs_gtp_xact_commit(xact);
     ogs_expect_or_return(rv == OGS_OK);
@@ -1127,6 +1136,15 @@ void mme_s11_handle_release_access_bearers_response(
      ***********************/
     if (!mme_ue_from_teid) {
         ogs_error("No Context in TEID");
+        mme_send_delete_session_or_mme_ue_context_release(mme_ue);
+        return;        
+    }
+
+    sgw_ue = sgw_ue_cycle(mme_ue->sgw_ue);
+    if (!sgw_ue) {
+        ogs_error("Cannot find sgw_ue");
+        mme_send_delete_session_or_mme_ue_context_release(mme_ue);
+        return;        
     }
 
     /********************

src/mme/nas-path.c

@@ -131,6 +131,7 @@ int nas_eps_send_attach_reject(mme_ue_t *mme_ue,
     ogs_nas_emm_cause_t emm_cause, ogs_nas_esm_cause_t esm_cause)
 {
     int rv;
+    enb_ue_t *enb_ue = NULL;
     mme_sess_t *sess = NULL;
     ogs_pkbuf_t *esmbuf = NULL, *emmbuf = NULL;
 
@@ -139,6 +140,12 @@ int nas_eps_send_attach_reject(mme_ue_t *mme_ue,
     ogs_debug("[%s] Attach reject", mme_ue->imsi_bcd);
     ogs_debug("    Cause[%d]", emm_cause);
 
+    enb_ue = enb_ue_cycle(mme_ue->enb_ue);
+    if (!enb_ue) {
+        ogs_error("S1 context has already been removed");
+        return OGS_OK;
+    }
+
     sess = mme_sess_first(mme_ue);
     if (sess) {
         esmbuf = esm_build_pdn_connectivity_reject(