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

1 files changed, 14 insertions, 10 deletions

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;