Change in libosmocore[master]: ns2: Avoid use-after-free when SGSN-side non-persistent SNS-NSE fails

[daniel]

daniel has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/26199 )


Change subject: ns2: Avoid use-after-free when SGSN-side non-persistent SNS-NSE fails
......................................................................

ns2: Avoid use-after-free when SGSN-side non-persistent SNS-NSE fails

alive_timeout_handler() changes the state to RECOVERING which calls
ns2_st_alive_onenter()->ns2_nse_notify_unblocked(unblocked=false)->
ns2_sns_notify_alive(unblocked=false)

When all (signalling) NSVCs have failed and gss->role is SGSN and not
persistent sns_failed() calls gprs_ns2_free_nse() which talloc_free()s
the nse before returning.

The next line in ns2_nse_notify_unblocked() tries to read nse->alive which then causes the
use-after-free.

Change-Id: I0486a77fd3e21fd3904bd19e4e0225ffbf654935
Related: OS#5302
---
M src/gb/gprs_ns2.c
M src/gb/gprs_ns2_internal.h
M src/gb/gprs_ns2_sns.c
3 files changed, 14 insertions(+), 11 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/99/26199/1

diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c
index 3bb04ae..c422cda 100644
--- a/src/gb/gprs_ns2.c
+++ b/src/gb/gprs_ns2.c
@@ -1391,12 +1391,13 @@
  *  \param[in] unblocked whether the NSE should be marked as unblocked (true) or blocked (false) */
 void ns2_nse_notify_unblocked(struct gprs_ns2_vc *nsvc, bool unblocked)
 {
+	int rc;
 	struct gprs_ns2_nse *nse = nsvc->nse;
 
 	ns2_nse_data_sum(nse);
-	ns2_sns_notify_alive(nse, nsvc, unblocked);
+	rc = ns2_sns_notify_alive(nse, nsvc, unblocked);
 
-	if (unblocked == nse->alive)
+	if (rc == -ENOENT || unblocked == nse->alive)
 		return;
 
 	/* wait until both data_weight and sig_weight are != 0 before declaring NSE as alive */
diff --git a/src/gb/gprs_ns2_internal.h b/src/gb/gprs_ns2_internal.h
index 0959d2b..aaf0897 100644
--- a/src/gb/gprs_ns2_internal.h
+++ b/src/gb/gprs_ns2_internal.h
@@ -456,7 +456,7 @@
 					     const char *id);
 struct osmo_fsm_inst *ns2_sns_sgsn_fsm_alloc(struct gprs_ns2_nse *nse, const char *id);
 void ns2_sns_replace_nsvc(struct gprs_ns2_vc *nsvc);
-void ns2_sns_notify_alive(struct gprs_ns2_nse *nse, struct gprs_ns2_vc *nsvc, bool alive);
+int ns2_sns_notify_alive(struct gprs_ns2_nse *nse, struct gprs_ns2_vc *nsvc, bool alive);
 void ns2_sns_update_weights(struct gprs_ns2_vc_bind *bind);
 
 /* vc */
diff --git a/src/gb/gprs_ns2_sns.c b/src/gb/gprs_ns2_sns.c
index 0afc06e..7984417 100644
--- a/src/gb/gprs_ns2_sns.c
+++ b/src/gb/gprs_ns2_sns.c
@@ -2584,24 +2584,24 @@
 	return count;
 }
 
-void ns2_sns_notify_alive(struct gprs_ns2_nse *nse, struct gprs_ns2_vc *nsvc, bool alive)
+int ns2_sns_notify_alive(struct gprs_ns2_nse *nse, struct gprs_ns2_vc *nsvc, bool alive)
 {
 	struct ns2_sns_state *gss;
 	struct gprs_ns2_vc *tmp;
 
 	if (!nse->bss_sns_fi)
-		return;
+		return 0;
 
 	gss = nse->bss_sns_fi->priv;
 	if (nse->bss_sns_fi->state != GPRS_SNS_ST_CONFIGURED && nse->bss_sns_fi->state != GPRS_SNS_ST_LOCAL_PROCEDURE)
-		return;
+		return 0;
 
 	if (gss->block_no_nsvc_events)
-		return;
+		return 0;
 
 	if (gss->alive && nse->sum_sig_weight == 0) {
 		sns_failed(nse->bss_sns_fi, "No signalling NSVC available");
-		return;
+		return -ENOENT;
 	}
 
 	/* check if this is the current SNS NS-VC */
@@ -2620,25 +2620,27 @@
 	}
 
 	if (alive == gss->alive)
-		return;
+		return 0;
 
 	if (alive) {
 		/* we need at least a signalling NSVC before become alive */
 		if (nse->sum_sig_weight == 0)
-			return;
+			return 0;
 		gss->alive = true;
 		osmo_fsm_inst_dispatch(nse->bss_sns_fi, NS2_SNS_EV_REQ_NSVC_ALIVE, NULL);
 	} else {
 		/* is there at least another alive nsvc? */
 		llist_for_each_entry(tmp, &nse->nsvc, list) {
 			if (ns2_vc_is_unblocked(tmp))
-				return;
+				return 0;
 		}
 
 		/* all NS-VC have failed */
 		gss->alive = false;
 		osmo_fsm_inst_dispatch(nse->bss_sns_fi, NS2_SNS_EV_REQ_NO_NSVC, NULL);
 	}
+
+	return 0;
 }
 
 int gprs_ns2_sns_add_bind(struct gprs_ns2_nse *nse,

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/26199
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0486a77fd3e21fd3904bd19e4e0225ffbf654935
Gerrit-Change-Number: 26199
Gerrit-PatchSet: 1
Gerrit-Owner: daniel <dwillmann at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211110/26d58a67/attachment.htm>