diff options
authorNeels Hofmeyr <neels@hofmeyr.de>2019-03-07 03:06:16 +0100
committerNeels Hofmeyr <neels@hofmeyr.de>2019-03-07 03:58:29 +0100
commit8e2c6a31c1401b5a6980866ef35d47eb3d8d5ca3 (patch)
parenta3a8821167b7b62c17a3002ddbd992eb05e92ab2 (diff)
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
2 files changed, 11 insertions, 3 deletions
diff --git a/src/libmsc/gsm_subscriber.c b/src/libmsc/gsm_subscriber.c
index e60344fec..c4faa948c 100644
--- a/src/libmsc/gsm_subscriber.c
+++ b/src/libmsc/gsm_subscriber.c
@@ -200,8 +200,14 @@ struct ran_conn *connection_for_subscr(struct vlr_subscr *vsub)
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 79709c63a..e54e542b9 100644
--- a/src/libmsc/ran_conn.c
+++ b/src/libmsc/ran_conn.c
@@ -538,8 +538,10 @@ static void ran_conn_fsm_cleanup(struct osmo_fsm_inst *fi, enum osmo_fsm_term_ca
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");