fix crashes due to OSMO_ASSERT(conn->lchan)

Starting from ttcn3-bsc-test-sccplite build #777, it was noticed
that osmo-bsc crashes with the following message:

  Assert failed conn->lchan include/osmocom/bsc/gsm_data.h:1376

The cause of this is a recently merged patch that calls conn_get_bts() during
assignment_fsm rate counter dispatch:
"Count assignment rates per BTS as well"
commit b5ccf09fc4042c7fb1fdaaa6263961c40b32564e
Change-Id I0009e51d4caf68e762138d98e2e23d49acc3cc1a

The root cause being that the assignment_fsm attempts to count an Assignment
event for a BTS after the lchan has already been released and disassociated
from the conn.

The assertion is found in conn_get_bts(), which is used in various places. In
fact, each caller is a potential DoS risk -- though most are in code paths that
are guaranteed to have an lchan and bts present, having an OSMO_ASSERT() on the
relatively volatile presence of an lchan is not a good idea for osmo-bsc's
stability and error resilience.

- Change conn_get_bts() to return NULL in the lack of an lchan.
- Adjust all callers of conn_get_bts() to gracefully handle a NULL return val.
- Same for cgi_for_msc() and callers, closely related.

Here is a backtrace:

  Program received signal SIGABRT
  pwndbg> bt
    0x0000555555be6e52 in conn_get_bts (conn=0x622000057160) at include/osmocom/bsc/gsm_data.h:1376
    0x0000555555c1edc8 in assignment_fsm_timer_cb (fi=0x612000060220) at assignment_fsm.c:758
    0x00007ffff72b1104 in fsm_tmr_cb (data=0x612000060220) at libosmocore/src/fsm.c:325
    0x00007ffff72ab062 in osmo_timers_update () at libosmocore/src/timer.c:257
    0x00007ffff72ab5d2 in _osmo_select_main (polling=0) at libosmocore/src/select.c:260
    0x00007ffff72abd2f in osmo_select_main_ctx (polling=<optimized out>) at libosmocore/src/select.c:291
    0x0000555555e1b81b in main (argc=3, argv=0x7fffffffe1b8) at osmo_bsc_main.c:953
    0x00007ffff6752002 in __libc_start_main () from /usr/lib/libc.so.6
    0x0000555555b61bbe in _start ()

In the case of the assignment_fsm counter, we now miss a chance to increase a
BTS counter for a failed Assignment, but this is a separate problem. The main
point of this patch is that osmo-bsc must not crash.

Related: OS#4620, OS#4619
Patch-by: fixeria
Tweaked-by: neels
Fixes: I0009e51d4caf68e762138d98e2e23d49acc3cc1a
Change-Id: Id681dfb0ad654bdb4b71805d1ad4f39a8bf6bbd1

9 files changed, 72 insertions, 25 deletions

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 8c1c54504..ce18d1b7e 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -1373,7 +1373,8 @@ bool ts_is_tch(struct gsm_bts_trx_ts *ts);
 
 
 static inline struct gsm_bts *conn_get_bts(struct gsm_subscriber_connection *conn) {
-	OSMO_ASSERT(conn->lchan);
+	if (!conn || !conn->lchan)
+		return NULL;
 	return conn->lchan->ts->trx->bts;
 }
 
diff --git a/src/osmo-bsc/assignment_fsm.c b/src/osmo-bsc/assignment_fsm.c
index 94dd35924..cd5abc857 100644
--- a/src/osmo-bsc/assignment_fsm.c
+++ b/src/osmo-bsc/assignment_fsm.c
@@ -80,7 +80,8 @@ static const struct osmo_tdef_state_timeout assignment_fsm_timeouts[32] = {
 			       bsc_ctr_description[BSC_##counter].name, \
 			       bsc_ctr_description[BSC_##counter].description); \
 		rate_ctr_inc(&conn->network->bsc_ctrs->ctr[BSC_##counter]); \
-		rate_ctr_inc(&bts->bts_ctrs->ctr[BTS_##counter]); \
+		if (bts) \
+			rate_ctr_inc(&bts->bts_ctrs->ctr[BTS_##counter]); \
 	} while(0)
 
 #define assignment_count_result(counter) do { \
diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index 9896bff71..2941e5b4f 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -1776,7 +1776,10 @@ static int ho_or_as(struct vty *vty, const char *argv[], int argc)
 
 	/* Find the connection/lchan that we want to handover */
 	llist_for_each_entry(conn, &net->subscr_conns, entry) {
-		if (conn_get_bts(conn)->nr == bts_nr &&
+		struct gsm_bts *bts = conn_get_bts(conn);
+		if (!bts)
+			continue;
+		if (bts->nr == bts_nr &&
 		    conn->lchan->ts->trx->nr == trx_nr &&
 		    conn->lchan->ts->nr == ts_nr && conn->lchan->nr == ss_nr) {
 			vty_out(vty, "starting %s for lchan %s...%s", action, conn->lchan->name, VTY_NEWLINE);
diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c
index 3703c76aa..e28850698 100644
--- a/src/osmo-bsc/gsm_08_08.c
+++ b/src/osmo-bsc/gsm_08_08.c
@@ -222,8 +222,9 @@ static struct bsc_msc_data *bsc_find_msc(struct gsm_subscriber_connection *conn,
 	/* Has the subscriber been paged from a connected MSC? */
 	if (pdisc == GSM48_PDISC_RR && mtype == GSM48_MT_RR_PAG_RESP) {
 		subscr = bsc_subscr_find_by_mi(conn->network->bsc_subscribers, &mi);
-		if (subscr) {
-			msc_target = paging_get_msc(conn_get_bts(conn), subscr);
+		struct gsm_bts *bts = conn_get_bts(conn);
+		if (bts && subscr) {
+			msc_target = paging_get_msc(bts, subscr);
 			bsc_subscr_put(subscr);
 			if (is_msc_usable(msc_target, is_emerg)) {
 				LOG_COMPL_L3(pdisc, mtype, LOGL_DEBUG, "%s matches earlier Paging from msc %d\n",
@@ -356,6 +357,13 @@ static int handle_page_resp(struct gsm_subscriber_connection *conn, struct msgb
 {
 	struct osmo_mobile_identity mi;
 	struct bsc_subscr *subscr = NULL;
+	struct gsm_bts *bts = conn_get_bts(conn);
+
+	if (!bts) {
+		/* should never happen */
+		LOGP(DRSL, LOGL_ERROR, "Paging Response without lchan\n");
+		return -1;
+	}
 
 	if (osmo_mobile_identity_decode_from_l3(&mi, msg, false)) {
 		LOGP(DRSL, LOGL_ERROR, "Unable to extract Mobile Identity from Paging Response\n");
@@ -370,8 +378,7 @@ static int handle_page_resp(struct gsm_subscriber_connection *conn, struct msgb
 		return -1;
 	}
 
-	paging_request_stop(&conn->network->bts_list, conn_get_bts(conn), subscr, conn,
-			    msg);
+	paging_request_stop(&conn->network->bts_list, bts, subscr, conn, msg);
 	bsc_subscr_put(subscr);
 	return 0;
 }
@@ -385,6 +392,11 @@ static void handle_lu_request(struct gsm_subscriber_connection *conn,
 	int8_t rc8;
 	struct gsm_bts *bts = conn_get_bts(conn);
 
+	if (!bts) {
+		/* should never happen */
+		LOGP(DRSL, LOGL_ERROR, "LU Request without lchan\n");
+		return;
+	}
 
 	if (msgb_l3len(msg) < sizeof(*gh) + sizeof(*lu)) {
 		LOGP(DMSC, LOGL_ERROR, "LU too small to look at: %u\n", msgb_l3len(msg));
@@ -415,6 +427,12 @@ static void handle_cm_serv_req(struct gsm_subscriber_connection *conn,
 	int8_t rc8;
 	struct gsm_bts *bts = conn_get_bts(conn);
 
+	if (!bts) {
+		/* should never happen */
+		LOGP(DRSL, LOGL_ERROR, "CM Service Request without lchan\n");
+		return;
+	}
+
 	if (msgb_l3len(msg) < sizeof(*gh) + sizeof(*serv_req)) {
 		LOGP(DMSC, LOGL_ERROR, "CM Serv Req too small to look at: %u\n", msgb_l3len(msg));
 		return;
@@ -462,6 +480,15 @@ int bsc_compl_l3(struct gsm_subscriber_connection *conn, struct msgb *msg, uint1
 	struct msgb *resp;
 	struct gsm0808_speech_codec_list scl;
 	int rc = -2;
+	struct gsm_bts *bts = conn_get_bts(conn);
+	struct osmo_cell_global_id *cgi = cgi_for_msc(conn->sccp.msc, bts);
+
+	if (!bts || !cgi) {
+		/* should never happen */
+		LOGP(DMSC, LOGL_ERROR, "Compl L3 without lchan\n");
+		rc = -1;
+		goto early_fail;
+	}
 
 	log_set_context(LOG_CTX_BSC_SUBSCR, conn->bsub);
 
@@ -482,9 +509,9 @@ int bsc_compl_l3(struct gsm_subscriber_connection *conn, struct msgb *msg, uint1
 	bsc_scan_bts_msg(conn, msg);
 
 	if (gscon_is_aoip(conn)) {
-		gen_bss_supported_codec_list(&scl, msc, conn_get_bts(conn));
+		gen_bss_supported_codec_list(&scl, msc, bts);
 		if (scl.len > 0)
-			resp = gsm0808_create_layer3_2(msg, cgi_for_msc(conn->sccp.msc, conn_get_bts(conn)), &scl);
+			resp = gsm0808_create_layer3_2(msg, cgi, &scl);
 		else {
 			/* Note: 3GPP TS 48.008 3.2.1.32, COMPLETE LAYER 3 INFORMATION clearly states that
 			 * Codec List (BSS Supported) shall be included, if the radio access network
@@ -492,10 +519,10 @@ int bsc_compl_l3(struct gsm_subscriber_connection *conn, struct msgb *msg, uint1
 			 * current configuration does not support any voice codecs, in those cases the
 			 * network does not support an IP based user plane interface, and therefore the
 			 * Codec List (BSS Supported) IE can be left out in those situations. */
-			resp = gsm0808_create_layer3_2(msg, cgi_for_msc(conn->sccp.msc, conn_get_bts(conn)), NULL);
+			resp = gsm0808_create_layer3_2(msg, cgi, NULL);
 		}
 	} else
-		resp = gsm0808_create_layer3_2(msg, cgi_for_msc(conn->sccp.msc, conn_get_bts(conn)), NULL);
+		resp = gsm0808_create_layer3_2(msg, cgi, NULL);
 
 	if (resp)
 		rc = osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_A_CONN_REQ, resp);
@@ -537,6 +564,12 @@ void bsc_cm_update(struct gsm_subscriber_connection *conn,
 	struct msgb *resp;
 	struct gsm_bts *bts = conn_get_bts(conn);
 
+	if (!bts) {
+		/* should never happen */
+		LOGP(DMSC, LOGL_ERROR, "Classmark Update without lchan\n");
+		return;
+	}
+
 	rc8 = osmo_gsm48_rfpowercap2powerclass(bts->band, cm2_parsed->pwr_lev);
 	if (rc8 < 0) {
 		LOGP(DMSC, LOGL_NOTICE,
diff --git a/src/osmo-bsc/gsm_data.c b/src/osmo-bsc/gsm_data.c
index 9bf6f826c..2098f1345 100644
--- a/src/osmo-bsc/gsm_data.c
+++ b/src/osmo-bsc/gsm_data.c
@@ -1721,7 +1721,7 @@ void conn_update_ms_power_class(struct gsm_subscriber_connection *conn, uint8_t
 
 	/* If there's an associated lchan, attempt to update its max power to be
 	   on track with band maximum values */
-	if (conn->lchan)
+	if (bts && conn->lchan)
 		lchan_update_ms_power_ctrl_level(conn->lchan, bts->ms_max_power);
 }
 
diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index 6b225e44e..2deb7f46b 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -478,7 +478,6 @@ static int bssmap_handle_cipher_mode(struct gsm_subscriber_connection *conn,
 				     struct msgb *msg, unsigned int payload_length)
 {
 	uint16_t len;
-	struct gsm_network *network = NULL;
 	const uint8_t *data;
 	struct tlv_parsed tp;
 	struct msgb *resp;
@@ -522,7 +521,6 @@ static int bssmap_handle_cipher_mode(struct gsm_subscriber_connection *conn,
 		goto reject;
 	}
 
-	network = conn_get_bts(conn)->network;
 	data = TLVP_VAL(&tp, GSM0808_IE_ENCRYPTION_INFORMATION);
 	enc_bits_msc = data[0];
 	enc_key = &data[1];
@@ -540,10 +538,10 @@ static int bssmap_handle_cipher_mode(struct gsm_subscriber_connection *conn,
 	/* The bit-mask of permitted ciphers from the MSC (sent in ASSIGNMENT COMMAND) is intersected
 	 * with the vty-configured mask a the BSC.  Finally, the best (highest) possible cipher is
 	 * chosen. */
-	chosen_cipher = select_best_cipher(enc_bits_msc, network->a5_encryption_mask);
+	chosen_cipher = select_best_cipher(enc_bits_msc, bsc_gsmnet->a5_encryption_mask);
 	if (chosen_cipher < 0) {
 		LOGP(DMSC, LOGL_ERROR, "Reject: no overlapping A5 ciphers between BSC (0x%02x) "
-			"and MSC (0x%02x)\n", network->a5_encryption_mask, enc_bits_msc);
+			"and MSC (0x%02x)\n", bsc_gsmnet->a5_encryption_mask, enc_bits_msc);
 		reject_cause = GSM0808_CAUSE_CIPHERING_ALGORITHM_NOT_SUPPORTED;
 		goto reject;
 	}
@@ -663,17 +661,23 @@ static int select_codecs(struct assignment_request *req, struct gsm0808_channel_
 {
 	int rc, i, nc = 0;
 	struct bsc_msc_data *msc;
+	struct gsm_bts *bts = conn_get_bts(conn);
+
+	if (!bts) {
+		LOGP(DMSC, LOGL_ERROR, "No lchan, cannot select codecs\n");
+		return -EINVAL;
+	}
 
 	msc = conn->sccp.msc;
 
 	switch (ct->ch_rate_type) {
 	case GSM0808_SPEECH_FULL_BM:
-		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, conn_get_bts(conn),
+		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, bts,
 				      RATE_PREF_FR);
 		nc += (rc == 0);
 		break;
 	case GSM0808_SPEECH_HALF_LM:
-		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, conn_get_bts(conn),
+		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, bts,
 				      RATE_PREF_HR);
 		nc += (rc == 0);
 		break;
@@ -681,19 +685,19 @@ static int select_codecs(struct assignment_request *req, struct gsm0808_channel_
 	case GSM0808_SPEECH_PERM_NO_CHANGE:
 	case GSM0808_SPEECH_FULL_PREF_NO_CHANGE:
 	case GSM0808_SPEECH_FULL_PREF:
-		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, conn_get_bts(conn),
+		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, bts,
 				      RATE_PREF_FR);
 		nc += (rc == 0);
-		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, conn_get_bts(conn),
+		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, bts,
 				      RATE_PREF_HR);
 		nc += (rc == 0);
 		break;
 	case GSM0808_SPEECH_HALF_PREF_NO_CHANGE:
 	case GSM0808_SPEECH_HALF_PREF:
-		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, conn_get_bts(conn),
+		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, bts,
 				      RATE_PREF_HR);
 		nc += (rc == 0);
-		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, conn_get_bts(conn),
+		rc = match_codec_pref(&req->ch_mode_rate[nc], ct, &conn->codec_list, msc, bts,
 				      RATE_PREF_FR);
 		nc += (rc == 0);
 		break;
diff --git a/src/osmo-bsc/osmo_bsc_filter.c b/src/osmo-bsc/osmo_bsc_filter.c
index 98b51485c..08c3a5048 100644
--- a/src/osmo-bsc/osmo_bsc_filter.c
+++ b/src/osmo-bsc/osmo_bsc_filter.c
@@ -132,11 +132,12 @@ int bsc_scan_msc_msg(struct gsm_subscriber_connection *conn, struct msgb *msg)
 	msc = conn->sccp.msc;
 
 	if (mtype == GSM48_MT_MM_LOC_UPD_ACCEPT) {
-		if (has_core_identity(msc)) {
+		struct gsm_bts *bts = conn_get_bts(conn);
+		if (bts && has_core_identity(msc)) {
 			if (msgb_l3len(msg) >= sizeof(*gh) + sizeof(*lai)) {
 				/* overwrite LAI in the message */
 				lai = (struct gsm48_loc_area_id *) &gh->data[0];
-				gsm48_generate_lai2(lai, bts_lai(conn_get_bts(conn)));
+				gsm48_generate_lai2(lai, bts_lai(bts));
 			}
 		}
 		return 0;
diff --git a/src/osmo-bsc/osmo_bsc_msc.c b/src/osmo-bsc/osmo_bsc_msc.c
index 9b00ffcb4..f9691462c 100644
--- a/src/osmo-bsc/osmo_bsc_msc.c
+++ b/src/osmo-bsc/osmo_bsc_msc.c
@@ -266,6 +266,10 @@ struct bsc_msc_data *osmo_msc_data_alloc(struct gsm_network *net, int nr)
 struct osmo_cell_global_id *cgi_for_msc(struct bsc_msc_data *msc, struct gsm_bts *bts)
 {
 	static struct osmo_cell_global_id cgi;
+
+	if (!bts)
+		return NULL;
+
 	cgi.lai.plmn = msc->network->plmn;
 	if (msc->core_plmn.mcc != GSM_MCC_MNC_INVALID)
 		cgi.lai.plmn.mcc = msc->core_plmn.mcc;
diff --git a/src/osmo-bsc/osmo_bsc_sigtran.c b/src/osmo-bsc/osmo_bsc_sigtran.c
index f5489e421..43ffae023 100644
--- a/src/osmo-bsc/osmo_bsc_sigtran.c
+++ b/src/osmo-bsc/osmo_bsc_sigtran.c
@@ -311,7 +311,7 @@ enum bsc_con osmo_bsc_sigtran_new_conn(struct gsm_subscriber_connection *conn, s
 		return BSC_CON_REJECT_NO_LINK;
 	}
 
-	if (!bsc_grace_allow_new_connection(bts->network, bts)) {
+	if (bts && !bsc_grace_allow_new_connection(bts->network, bts)) {
 		LOGP(DMSC, LOGL_NOTICE, "BSC in grace period. No new connections.\n");
 		return BSC_CON_REJECT_RF_GRACE;
 	}