bssap: Speculative crash fix when queueing messages for the BTS

It appears to be possible that we attempt to submit a DTAP
on a SCCP connection when we have a channel without the msc_data
assigned. This change should fix the crash (which is not well
understood), fix a memleak in the case of the queue being full.

3 files changed, 30 insertions, 6 deletions

diff --git a/openbsc/include/openbsc/bssap.h b/openbsc/include/openbsc/bssap.h
index 3b06fb5c3..f32daf006 100644
--- a/openbsc/include/openbsc/bssap.h
+++ b/openbsc/include/openbsc/bssap.h
@@ -326,7 +326,6 @@ void bsc_queue_connection_write(struct sccp_connection *conn, struct msgb *msg);
 void bsc_free_queued(struct sccp_connection *conn);
 void bsc_send_queued(struct sccp_connection *conn);
 
-void bts_queue_send(struct msgb *msg, int link_id);
 void bts_send_queued(struct bss_sccp_connection_data*);
 void bts_free_queued(struct bss_sccp_connection_data*);
 void bts_unblock_queue(struct bss_sccp_connection_data*);
diff --git a/openbsc/src/bsc_msc_ip.c b/openbsc/src/bsc_msc_ip.c
index 18cfbeba9..71b7884b7 100644
--- a/openbsc/src/bsc_msc_ip.c
+++ b/openbsc/src/bsc_msc_ip.c
@@ -136,21 +136,35 @@ struct gsm_subscriber *find_subscriber(u_int8_t type, const char *mi_string)
 /* SCCP handling */
 void msc_outgoing_sccp_data(struct sccp_connection *conn, struct msgb *msg, unsigned int len)
 {
+	struct gsm_lchan *lchan;
 	struct bssmap_header *bs;
 
 	if (len < 1) {
-		DEBUGP(DMSC, "The header is too short.\n");
+		LOGP(DMSC, LOGL_ERROR, "The header is too short.\n");
+		return;
+	}
+
+	lchan = sccp_get_lchan(conn->data_ctx);
+	if (!lchan) {
+		LOGP(DMSC, LOGL_ERROR, "SCCP data without lchan for type: 0x%x\n", msg->l3h[0]);
+		return;
+	}
+
+	/* that is bad */
+	if (!lchan->msc_data) {
+		LOGP(DMSC, LOGL_ERROR, "SCCP data for lchan without msc data type: 0x%x\n",
+		     msg->l3h[0]);
 		return;
 	}
 
 	switch (msg->l3h[0]) {
 	case BSSAP_MSG_BSS_MANAGEMENT:
 		msg->l4h = &msg->l3h[sizeof(*bs)];
-		msg->lchan = sccp_get_lchan(conn->data_ctx);
+		msg->lchan = lchan;
 		bssmap_rcvmsg_dt1(conn, msg, len - sizeof(*bs));
 		break;
 	case BSSAP_MSG_DTAP:
-		dtap_rcvmsg(sccp_get_lchan(conn->data_ctx), msg, len);
+		dtap_rcvmsg(lchan, msg, len);
 		break;
 	default:
 		DEBUGPC(DMSC, "Unimplemented msg type: %d\n", msg->l3h[0]);
diff --git a/openbsc/src/bssap.c b/openbsc/src/bssap.c
index ff682e965..ee51a392f 100644
--- a/openbsc/src/bssap.c
+++ b/openbsc/src/bssap.c
@@ -40,6 +40,8 @@
 #define BSSMAP_MSG_SIZE 512
 #define BSSMAP_MSG_HEADROOM 128
 
+static void bts_queue_send(struct msgb *msg, int link_id);
+
 
 static const struct tlv_definition bss_att_tlvdef = {
 	.def = {
@@ -1202,9 +1204,17 @@ static void rll_ind_cb(struct gsm_lchan *lchan, u_int8_t link_id,
 }
 
 /* decide if we need to queue because of SAPI != 0 */
-void bts_queue_send(struct msgb *msg, int link_id)
+static void bts_queue_send(struct msgb *msg, int link_id)
 {
-	struct bss_sccp_connection_data *data = msg->lchan->msc_data;
+
+	struct bss_sccp_connection_data *data;
+
+	if (!msg->lchan || !msg->lchan->msc_data) {
+		LOGP(DMSC, LOGL_ERROR, "BAD: Wrongly configured lchan: %p\n", msg->lchan);
+		msgb_free(msg);
+	}
+
+	data = msg->lchan->msc_data;
 
 	if (!data->block_gsm && data->gsm_queue_size == 0) {
 		if (msg->lchan->sapis[link_id & 0x7] != LCHAN_SAPI_UNUSED) {
@@ -1221,6 +1231,7 @@ void bts_queue_send(struct msgb *msg, int link_id)
 		}
 	} else if (data->gsm_queue_size == 10) {
 		LOGP(DMSC, LOGL_ERROR, "Queue full on %p. Dropping GSM0408.\n", data->sccp);
+		msgb_free(msg);
 	} else {
 		LOGP(DMSC, LOGL_DEBUG, "Queueing GSM0408 message on %p. Queue size: %d\n",
 		       data->sccp, data->gsm_queue_size + 1);