libmsc: Avoid a crash on lchan release during call control

If subscriber A is calling B and has sent a CC Setup message we will
allocate the MO and MT transaction and link them together. When the
BTS or the lchan is failing the BSC API will send a clear request,
as part of the clear request all pending transactions will be released.

As part of taking down the transaction, the remote leg will be informed
and will send a MNCC_REL_REQ. This results in a call to trans_free. The
llist_for_each_entry_safe does not handle removing other elements from
the list and we would segfault.

One way to fix this is to move the transaction list into the subscriber
connection. This might require to create the subscriber connection for
MT handling earlier. Otherwise one could have one transaction list inside
the subscriber connection and a global list for MT- transactions.

1 files changed, 16 insertions, 5 deletions

diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c
index fd482e0bd..d1dfa4a72 100644
--- a/openbsc/src/libmsc/gsm_04_08.c
+++ b/openbsc/src/libmsc/gsm_04_08.c
@@ -339,13 +339,24 @@ void gsm0408_clear_request(struct gsm_subscriber_connection *conn, uint32_t caus
 	release_security_operation(conn);
 	release_anchor(conn);
 
-	/* Free all transactions that are associated with the released lchan */
-	/* FIXME: this is not neccessarily the right thing to do, we should
-	 * only set trans->lchan to NULL and wait for another lchan to be
-	 * established to the same MM entity (phone/subscriber) */
+	/*
+	 * Free all transactions that are associated with the released
+	 * connection. The transaction code will inform the CC or SMS
+	 * facilities that will send the release indications. As part of
+	 * the CC REL_IND the remote leg might be released and this will
+	 * trigger the call to trans_free. This is something the llist
+	 * macro can not handle and we will need to re-iterate the list.
+	 *
+	 * TODO: Move the trans_list into the subscriber connection and
+	 * create a pending list for MT transactions. These exist before
+	 * we have a subscriber connection.
+	 */
+restart:
 	llist_for_each_entry_safe(trans, temp, &conn->bts->network->trans_list, entry) {
-		if (trans->conn == conn)
+		if (trans->conn == conn) {
 			trans_free(trans);
+			goto restart;
+		}
 	}
 }