From 1252568d326033c471c644037c6a9eb7a8d1ce83 Mon Sep 17 00:00:00 2001
From: Spencer Sevilla <spencer.builds.networks@gmail.com>
Date: Mon, 30 Jan 2023 15:56:08 -0800
Subject: [PATCH] [mme] turn various asserts into checks with error-handling
 (#69)

---
 README.md                 |  3 +++
 src/mme/mme-s11-handler.c | 46 +++++++++++++++++++++++++++------------
 src/mme/nas-path.c        |  7 ++++++
 3 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/README.md b/README.md
index a03e0a2d2a..9737a98eaf 100644
--- a/README.md
+++ b/README.md
@@ -35,3 +35,6 @@ When a UPS re-associates itself with the CPS after a period of disconnectivity,
 
 ## 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 keep this parameter as the default, but have also added 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 supplemented 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.
diff --git a/src/mme/mme-s11-handler.c b/src/mme/mme-s11-handler.c
index f31a4fb63e..29bd1153f7 100644
--- a/src/mme/mme-s11-handler.c
+++ b/src/mme/mme-s11-handler.c
@@ -124,16 +124,6 @@ void mme_s11_handle_create_session_response(
         ogs_error("MME-UE Context has already been removed");
         return;
     }
-    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);
-    }
 
     /************************
      * Getting Cause Value
@@ -155,6 +145,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 [Cause:%d]",
@@ -498,8 +505,6 @@ void mme_s11_handle_modify_bearer_response(
         ogs_error("MME-UE Context has already been removed");
         return;
     }
-    sgw_ue = sgw_ue_cycle(mme_ue->sgw_ue);
-    ogs_assert(sgw_ue);
 
     /************************
      * Getting Cause Value
@@ -521,6 +526,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;
@@ -1264,14 +1275,21 @@ void mme_s11_handle_release_access_bearers_response(
         ogs_error("MME-UE Context has already been removed");
         return;
     }
-    sgw_ue = sgw_ue_cycle(mme_ue->sgw_ue);
-    ogs_assert(sgw_ue);
 
     /***********************
      * Check MME-UE Context
      ***********************/
     if (!mme_ue_from_teid) {
         ogs_error("No Context in TEID [ACTION:%d]", action);
+        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;        
     }
 
     /********************
diff --git a/src/mme/nas-path.c b/src/mme/nas-path.c
index 36836e0e4f..b53aaa8050 100644
--- a/src/mme/nas-path.c
+++ b/src/mme/nas-path.c
@@ -184,6 +184,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;
 
@@ -200,6 +201,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(