Change in osmo-msc[master]: use only accepted ran_conns for new transactions

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Thu Mar 7 15:41:47 UTC 2019


Neels Hofmeyr has submitted this change and it was merged. ( https://gerrit.osmocom.org/13152 )

Change subject: use only accepted ran_conns for new transactions
......................................................................

use only accepted ran_conns for new transactions

In connection_for_subscriber(), do not return a ran_conn that is not yet
authenticated nor one that is already in release.

Using a ran_conn that is not yet authenticated may cause an auth/ciph
violation.

Using a ran_conn that is already in release may cause a use-after-free, see
OS#3842 for a description.

To be paranoid, upon releasing a conn, go through the transaction freeing
motions again by calling trans_conn_closed(), just in case some odd code path
added another transaction while the conn was already in release.

Related: OS#3842
Change-Id: Id957032e0ae1ff8ba055a75c3523447d3d06cbc3
---
M src/libmsc/gsm_subscriber.c
M src/libmsc/ran_conn.c
2 files changed, 11 insertions(+), 3 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Max: Looks good to me, but someone else must approve
  Vadim Yanitskiy: Looks good to me, but someone else must approve
  Pau Espin Pedrol: Looks good to me, approved



diff --git a/src/libmsc/gsm_subscriber.c b/src/libmsc/gsm_subscriber.c
index e60344f..c4faa94 100644
--- a/src/libmsc/gsm_subscriber.c
+++ b/src/libmsc/gsm_subscriber.c
@@ -200,8 +200,14 @@
 	struct ran_conn *conn;
 
 	llist_for_each_entry(conn, &net->ran_conns, entry) {
-		if (conn->vsub == vsub)
-			return conn;
+		if (conn->vsub != vsub)
+			continue;
+		/* Found a conn, but is it in a usable state? Must not add transactions to a conn that is in release,
+		 * and must not start transactions for an unauthenticated subscriber. There will obviously be only one
+		 * conn for this vsub, so return NULL right away. */
+		if (!ran_conn_is_accepted(conn))
+			return NULL;
+		return conn;
 	}
 
 	return NULL;
diff --git a/src/libmsc/ran_conn.c b/src/libmsc/ran_conn.c
index 79709c6..e54e542 100644
--- a/src/libmsc/ran_conn.c
+++ b/src/libmsc/ran_conn.c
@@ -538,8 +538,10 @@
 {
 	struct ran_conn *conn = fi->priv;
 
-	if (ran_conn_fsm_has_active_transactions(fi))
+	if (ran_conn_fsm_has_active_transactions(fi)) {
 		LOGPFSML(fi, LOGL_ERROR, "Deallocating despite active transactions\n");
+		trans_conn_closed(conn);
+	}
 
 	if (!conn) {
 		LOGP(DRLL, LOGL_ERROR, "Freeing NULL RAN connection\n");

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id957032e0ae1ff8ba055a75c3523447d3d06cbc3
Gerrit-Change-Number: 13152
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190307/e34e10fd/attachment.html>


More information about the gerrit-log mailing list