Change in osmo-bsc[master]: RSL chan_nr: replace OSMO_ASSERT with error handling

neels gerrit-no-reply at lists.osmocom.org
Wed Jun 2 21:20:06 UTC 2021


neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/24524 )


Change subject: RSL chan_nr: replace OSMO_ASSERT with error handling
......................................................................

RSL chan_nr: replace OSMO_ASSERT with error handling

It's bad to abort the program for an incompatible chan_nr.  Instead of
OSMO_ASSERT(), make sure that error handling happens all they way to the
original callers of gsm_lchan2chan_nr etc.

This is also preparation to add further error causes: Osmocom specific
cbits needed for a non-Osmo BTS.

Related: SYS#5315 OS#4940
Change-Id: I71ed6437c403a3f9336e17a94b4948fca295d853
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/abis_rsl.c
M src/osmo-bsc/gsm_04_08_rr.c
M src/osmo-bsc/gsm_data.c
M src/osmo-bsc/lcs_loc_req.c
M src/osmo-bsc/osmo_bsc_lcls.c
M src/osmo-bsc/system_information.c
M tests/handover/handover_test.c
8 files changed, 184 insertions(+), 60 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/24/24524/1

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 800dcf5..ce56952 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -1027,17 +1027,17 @@
 gsm_objclass2obj(struct gsm_bts *bts, uint8_t obj_class,
 	     const struct abis_om_obj_inst *obj_inst);
 
-uint8_t gsm_pchan2chan_nr(enum gsm_phys_chan_config pchan,
-			  uint8_t ts_nr, uint8_t lchan_nr);
-uint8_t gsm_lchan2chan_nr(const struct gsm_lchan *lchan);
-uint8_t gsm_lchan_as_pchan2chan_nr(const struct gsm_lchan *lchan,
-				   enum gsm_phys_chan_config as_pchan);
+int gsm_pchan2chan_nr(enum gsm_phys_chan_config pchan,
+		      uint8_t ts_nr, uint8_t lchan_nr);
+int gsm_lchan2chan_nr(const struct gsm_lchan *lchan);
+int gsm_lchan_as_pchan2chan_nr(const struct gsm_lchan *lchan,
+			       enum gsm_phys_chan_config as_pchan);
 
-void gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
-			   const struct gsm_lchan *lchan,
-			   uint8_t tsc);
-void gsm48_lchan2chan_desc_as_configured(struct gsm48_chan_desc *cd, const struct gsm_lchan *lchan,
-					 uint8_t tsc);
+int gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
+			  const struct gsm_lchan *lchan,
+			  uint8_t tsc);
+int gsm48_lchan2chan_desc_as_configured(struct gsm48_chan_desc *cd, const struct gsm_lchan *lchan,
+					uint8_t tsc);
 
 uint8_t gsm_ts_tsc(const struct gsm_bts_trx_ts *ts);
 
diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index c7e9821..fe7c480 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -279,8 +279,12 @@
 			  const uint8_t *data, int len)
 {
 	struct abis_rsl_dchan_hdr *dh;
-	struct msgb *msg = rsl_msgb_alloc();
-	uint8_t chan_nr = gsm_lchan2chan_nr(lchan);
+	struct msgb *msg;
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
+
+	msg = rsl_msgb_alloc();
 
 	dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh));
 	init_dchan_hdr(dh, RSL_MT_SACCH_INFO_MODIFY);
@@ -299,7 +303,9 @@
 {
 	struct abis_rsl_dchan_hdr *dh;
 	struct msgb *msg;
-	uint8_t chan_nr = gsm_lchan2chan_nr(lchan);
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
 
 	db = abs(db);
 	if (db > 30)
@@ -329,7 +335,9 @@
 {
 	struct abis_rsl_dchan_hdr *dh;
 	struct msgb *msg;
-	uint8_t chan_nr = gsm_lchan2chan_nr(lchan);
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
 
 	LOG_LCHAN(lchan, LOGL_DEBUG, "Tx MS POWER CONTROL (ms_power_lvl=%" PRIu8 ")\n",
 		  lchan->ms_power);
@@ -519,6 +527,9 @@
 
 	struct rsl_ie_chan_mode cm;
 	struct gsm48_chan_desc cd;
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
 
 	DEBUGP(DRSL, "%s Tx RSL Channel Activate with act_type=%s\n",
 	       gsm_ts_and_pchan_name(lchan->ts),
@@ -536,13 +547,17 @@
 	}
 
 	memset(&cd, 0, sizeof(cd));
-	gsm48_lchan2chan_desc(&cd, lchan, lchan->activate.tsc);
+	rc = gsm48_lchan2chan_desc(&cd, lchan, lchan->activate.tsc);
+	if (rc) {
+		LOG_LCHAN(lchan, LOGL_ERROR, "Error encoding Channel Number\n");
+		return rc;
+	}
 
 	msg = rsl_msgb_alloc();
 	dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh));
 	init_dchan_hdr(dh, RSL_MT_CHAN_ACTIV);
 
-	dh->chan_nr = gsm_lchan2chan_nr(lchan);
+	dh->chan_nr = chan_nr;
 
 	msgb_tv_put(msg, RSL_IE_ACT_TYPE, act_type);
 	msgb_tlv_put(msg, RSL_IE_CHAN_MODE, sizeof(cm),
@@ -648,10 +663,13 @@
 	struct msgb *msg;
 	int rc;
 
-	uint8_t chan_nr = gsm_lchan2chan_nr(lchan);
 	struct rsl_ie_chan_mode cm;
 	struct gsm_bts *bts = lchan->ts->trx->bts;
 
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
+
 	rc = channel_mode_from_lchan(&cm, lchan, &lchan->modify.ch_mode_rate, lchan->modify.info.vamos);
 	if (rc < 0)
 		return rc;
@@ -703,11 +721,14 @@
 {
 	struct abis_rsl_dchan_hdr *dh;
 	struct gsm_lchan *lchan = msg->lchan;
-	uint8_t chan_nr = gsm_lchan2chan_nr(lchan);
 	uint8_t encr_info[MAX_A5_KEY_LEN+2];
 	uint8_t l3_len = msg->len;
 	int rc;
 
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
+
 	/* First push the L3 IE tag and length */
 	msgb_tv16_push(msg, RSL_IE_L3_INFO, l3_len);
 
@@ -736,9 +757,13 @@
 	struct abis_rsl_dchan_hdr *dh;
 	struct msgb *msg = rsl_msgb_alloc();
 
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
+
 	dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh));
 	init_dchan_hdr(dh, RSL_MT_DEACTIVATE_SACCH);
-	dh->chan_nr = gsm_lchan2chan_nr(lchan);
+	dh->chan_nr = chan_nr;
 
 	msg->lchan = lchan;
 	msg->dst = rsl_chan_link(lchan);
@@ -754,10 +779,14 @@
 	struct abis_rsl_dchan_hdr *dh;
 	struct msgb *msg;
 
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
+
 	msg = rsl_msgb_alloc();
 	dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh));
 	init_dchan_hdr(dh, RSL_MT_RF_CHAN_REL);
-	dh->chan_nr = gsm_lchan2chan_nr(lchan);
+	dh->chan_nr = chan_nr;
 
 	msg->lchan = lchan;
 	msg->dst = rsl_chan_link(lchan);
@@ -886,13 +915,19 @@
 /* Send Siemens specific MS RF Power Capability Indication */
 int rsl_siemens_mrpci(struct gsm_lchan *lchan, struct rsl_mrpci *mrpci)
 {
-	struct msgb *msg = rsl_msgb_alloc();
+	struct msgb *msg;
 	struct abis_rsl_dchan_hdr *dh;
 
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
+
+	msg = rsl_msgb_alloc();
+
 	dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh));
 	init_dchan_hdr(dh, RSL_MT_SIEMENS_MRPCI);
 	dh->c.msg_discr = ABIS_RSL_MDISC_DED_CHAN;
-	dh->chan_nr = gsm_lchan2chan_nr(lchan);
+	dh->chan_nr = chan_nr;
 	msgb_tv_put(msg, RSL_IE_SIEMENS_MRPCI, *(uint8_t *)mrpci);
 
 	DEBUGP(DRSL, "%s TX Siemens MRPCI 0x%02x\n",
@@ -908,13 +943,19 @@
 /* Chapter 8.3.1 */
 int rsl_data_request(struct msgb *msg, uint8_t link_id)
 {
+	int chan_nr = gsm_lchan2chan_nr(msg->lchan);
+	if (chan_nr < 0) {
+		msgb_free(msg);
+		return chan_nr;
+	}
+
 	if (msg->lchan == NULL) {
 		LOGP(DRSL, LOGL_ERROR, "cannot send DATA REQUEST to unknown lchan\n");
+		msgb_free(msg);
 		return -EINVAL;
 	}
 
-	rsl_rll_push_l3(msg, RSL_MT_DATA_REQ, gsm_lchan2chan_nr(msg->lchan),
-			link_id, 1);
+	rsl_rll_push_l3(msg, RSL_MT_DATA_REQ, chan_nr, link_id, 1);
 
 	msg->dst = rsl_chan_link(msg->lchan);
 
@@ -926,9 +967,11 @@
 int rsl_establish_request(struct gsm_lchan *lchan, uint8_t link_id)
 {
 	struct msgb *msg;
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
 
-	msg = rsl_rll_simple(RSL_MT_EST_REQ, gsm_lchan2chan_nr(lchan),
-			     link_id, 0);
+	msg = rsl_rll_simple(RSL_MT_EST_REQ, chan_nr, link_id, 0);
 	msg->dst = rsl_chan_link(lchan);
 
 	DEBUGP(DRLL, "%s RSL RLL ESTABLISH REQ (link_id=0x%02x)\n",
@@ -947,9 +990,11 @@
 {
 
 	struct msgb *msg;
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
 
-	msg = rsl_rll_simple(RSL_MT_REL_REQ, gsm_lchan2chan_nr(lchan),
-			     link_id, 0);
+	msg = rsl_rll_simple(RSL_MT_REL_REQ, chan_nr, link_id, 0);
 	/* 0 is normal release, 1 is local end */
 	msgb_tv_put(msg, RSL_IE_RELEASE_MODE, release_mode);
 
@@ -1836,7 +1881,11 @@
 	ia->proto_discr = GSM48_PDISC_RR;
 	ia->msg_type = GSM48_MT_RR_IMM_ASS;
 	ia->page_mode = GSM48_PM_SAME;
-	gsm48_lchan2chan_desc(&ia->chan_desc, lchan, lchan->tsc);
+	rc = gsm48_lchan2chan_desc(&ia->chan_desc, lchan, lchan->tsc);
+	if (rc) {
+		LOG_LCHAN(lchan, LOGL_ERROR, "Error encoding Channel Number\n");
+		return rc;
+	}
 
 	/* use request reference extracted from CHAN_RQD */
 	memcpy(&ia->req_ref, lchan->rqd_ref, sizeof(ia->req_ref));
@@ -2275,13 +2324,19 @@
  */
 int rsl_tx_ipacc_crcx(const struct gsm_lchan *lchan)
 {
-	struct msgb *msg = rsl_msgb_alloc();
+	struct msgb *msg;
 	struct abis_rsl_dchan_hdr *dh;
 
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return chan_nr;
+
+	msg = rsl_msgb_alloc();
+
 	dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh));
 	init_dchan_hdr(dh, RSL_MT_IPAC_CRCX);
 	dh->c.msg_discr = ABIS_RSL_MDISC_IPACCESS;
-	dh->chan_nr = gsm_lchan2chan_nr(lchan);
+	dh->chan_nr = chan_nr;
 
 	/* 0x1- == receive-only, 0x-1 == EFR codec */
 	msgb_tv_put(msg, RSL_IE_IPAC_SPEECH_MODE, lchan->abis_ip.speech_mode);
@@ -2302,14 +2357,20 @@
  */
 struct msgb *rsl_make_ipacc_mdcx(const struct gsm_lchan *lchan, uint32_t dest_ip, uint16_t dest_port)
 {
-	struct msgb *msg = rsl_msgb_alloc();
+	struct msgb *msg;
 	struct abis_rsl_dchan_hdr *dh;
 	uint32_t *att_ip;
 
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0)
+		return NULL;
+
+	msg = rsl_msgb_alloc();
+
 	dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh));
 	init_dchan_hdr(dh, RSL_MT_IPAC_MDCX);
 	dh->c.msg_discr = ABIS_RSL_MDISC_IPACCESS;
-	dh->chan_nr = gsm_lchan2chan_nr(lchan);
+	dh->chan_nr = chan_nr;
 
 	msgb_tv16_put(msg, RSL_IE_IPAC_CONN_ID, lchan->abis_ip.conn_id);
 	msgb_v_put(msg, RSL_IE_IPAC_REMOTE_IP);
@@ -2334,6 +2395,9 @@
 {
 	struct msgb *msg = rsl_make_ipacc_mdcx(lchan, lchan->abis_ip.connect_ip, lchan->abis_ip.connect_port);
 
+	if (!msg)
+		return -EINVAL;
+
 	LOG_LCHAN(lchan, LOGL_DEBUG, "Sending IPACC MDCX to BTS:"
 		  " %s:%u rtp_payload=%u rtp_payload2=%u conn_id=%u speech_mode=0x%02x\n",
 		  ip_to_a(lchan->abis_ip.connect_ip),
@@ -2525,13 +2589,19 @@
 /*! Tx simplified channel (de-)activation message for non-standard ip.access dyn TS PDCH type. */
 static int send_ipacc_style_pdch_act(struct gsm_bts_trx_ts *ts, bool activate)
 {
-	struct msgb *msg = rsl_msgb_alloc();
+	struct msgb *msg;
 	struct abis_rsl_dchan_hdr *dh;
 
+	int chan_nr = gsm_pchan2chan_nr(GSM_PCHAN_TCH_F, ts->nr, 0);
+	if (chan_nr < 0)
+		return chan_nr;
+
+	msg = rsl_msgb_alloc();
+
 	dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh));
 	init_dchan_hdr(dh, activate ? RSL_MT_IPAC_PDCH_ACT : RSL_MT_IPAC_PDCH_DEACT);
 	dh->c.msg_discr = ABIS_RSL_MDISC_DED_CHAN;
-	dh->chan_nr = gsm_pchan2chan_nr(GSM_PCHAN_TCH_F, ts->nr, 0);
+	dh->chan_nr = chan_nr;
 
 	msg->dst = ts->trx->rsl_link_primary;
 	return abis_rsl_sendmsg(msg);
diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c
index 7cc5664..dc6434b 100644
--- a/src/osmo-bsc/gsm_04_08_rr.c
+++ b/src/osmo-bsc/gsm_04_08_rr.c
@@ -546,7 +546,10 @@
 
 	/* mandatory bits */
 	gsm48_cell_desc(&ho->cell_desc, new_lchan->ts->trx->bts);
-	gsm48_lchan2chan_desc(&ho->chan_desc, new_lchan, gsm_ts_tsc(new_lchan->ts));
+	if (gsm48_lchan2chan_desc(&ho->chan_desc, new_lchan, gsm_ts_tsc(new_lchan->ts))) {
+		msgb_free(msg);
+		return NULL;
+	}
 	ho->ho_ref = ho_ref;
 	ho->power_command = power_command;
 
@@ -597,6 +600,7 @@
 /* Chapter 9.1.2: Assignment Command */
 int gsm48_send_rr_ass_cmd(struct gsm_lchan *current_lchan, struct gsm_lchan *new_lchan, uint8_t power_command)
 {
+	int rc;
 	struct msgb *msg = gsm48_msgb_alloc_name("GSM 04.08 ASS CMD");
 	struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_put(msg, sizeof(*gh));
 	struct gsm48_ass_cmd *ass =
@@ -618,7 +622,11 @@
 	 * the chan_desc. But as long as multi-slot configurations
 	 * are not used we seem to be fine.
 	 */
-	gsm48_lchan2chan_desc(&ass->chan_desc, new_lchan, new_lchan->tsc);
+	rc = gsm48_lchan2chan_desc(&ass->chan_desc, new_lchan, new_lchan->tsc);
+	if (rc) {
+		msgb_free(msg);
+		return rc;
+	}
 	ass->power_command = power_command;
 
 	/* Cell Channel Description (freq. hopping), TV (see 3GPP TS 44.018, 10.5.2.1b) */
@@ -682,6 +690,7 @@
 /* 9.1.5 Channel mode modify: Modify the mode on the MS side */
 int gsm48_lchan_modify(struct gsm_lchan *lchan, uint8_t mode)
 {
+	int rc;
 	struct msgb *msg = gsm48_msgb_alloc_name("GSM 04.08 CHN MOD");
 	struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_put(msg, sizeof(*gh));
 	struct gsm48_chan_mode_modify *cmm =
@@ -694,7 +703,11 @@
 	gh->proto_discr = GSM48_PDISC_RR;
 	gh->msg_type = GSM48_MT_RR_CHAN_MODE_MODIF;
 
-	gsm48_lchan2chan_desc(&cmm->chan_desc, lchan, lchan->modify.tsc);
+	rc = gsm48_lchan2chan_desc(&cmm->chan_desc, lchan, lchan->modify.tsc);
+	if (rc) {
+		msgb_free(msg);
+		return rc;
+	}
 	cmm->mode = mode;
 
 	/* in case of multi rate we need to attach a config */
diff --git a/src/osmo-bsc/gsm_data.c b/src/osmo-bsc/gsm_data.c
index c43fbe9..ecbf7b9 100644
--- a/src/osmo-bsc/gsm_data.c
+++ b/src/osmo-bsc/gsm_data.c
@@ -495,23 +495,26 @@
 }
 
 /* See Table 10.5.25 of GSM04.08 */
-uint8_t gsm_pchan2chan_nr(enum gsm_phys_chan_config pchan,
-			  uint8_t ts_nr, uint8_t lchan_nr)
+int gsm_pchan2chan_nr(enum gsm_phys_chan_config pchan,
+		      uint8_t ts_nr, uint8_t lchan_nr)
 {
 	uint8_t cbits, chan_nr;
 
 	switch (pchan) {
 	case GSM_PCHAN_TCH_F:
 	case GSM_PCHAN_TCH_F_PDCH:
-		OSMO_ASSERT(lchan_nr == 0);
+		if (lchan_nr != 0)
+			return -EINVAL;
 		cbits = 0x01;
 		break;
 	case GSM_PCHAN_PDCH:
-		OSMO_ASSERT(lchan_nr == 0);
+		if (lchan_nr != 0)
+			return -EINVAL;
 		cbits = RSL_CHAN_OSMO_PDCH >> 3;
 		break;
 	case GSM_PCHAN_TCH_H:
-		OSMO_ASSERT(lchan_nr < 2);
+		if (lchan_nr > 1)
+			return -EINVAL;
 		cbits = 0x02;
 		cbits += lchan_nr;
 		break;
@@ -525,19 +528,21 @@
 		if (lchan_nr == CCCH_LCHAN)
 			chan_nr = 0;
 		else
-			OSMO_ASSERT(lchan_nr < 4);
+			return -EINVAL;
 		cbits = 0x04;
 		cbits += lchan_nr;
 		break;
 	case GSM_PCHAN_SDCCH8_SACCH8C:
 	case GSM_PCHAN_SDCCH8_SACCH8C_CBCH:
-		OSMO_ASSERT(lchan_nr < 8);
+		if (lchan_nr > 7)
+			return -EINVAL;
 		cbits = 0x08;
 		cbits += lchan_nr;
 		break;
 	default:
 	case GSM_PCHAN_CCCH:
-		OSMO_ASSERT(lchan_nr == 0);
+		if (lchan_nr != 0)
+			return -EINVAL;
 		cbits = 0x10;
 		break;
 	}
@@ -547,15 +552,23 @@
 	return chan_nr;
 }
 
-uint8_t gsm_lchan2chan_nr(const struct gsm_lchan *lchan)
+int gsm_lchan2chan_nr(const struct gsm_lchan *lchan)
 {
+	int rc;
 	uint8_t lchan_nr = lchan->nr;
 	/* The VAMOS lchans are behind the primary ones in the ts->lchan[] array. They keep their lchan->nr as in the
 	 * array, but on the wire they are the "shadow" lchans for the primary lchans. For example, for TCH/F, there is
 	 * a primary ts->lchan[0] and a VAMOS ts->lchan[1]. Still, the VAMOS lchan should send chan_nr = 0. */
 	if (lchan->vamos.is_secondary)
 		lchan_nr -= lchan->ts->max_primary_lchans;
-	return gsm_pchan2chan_nr(lchan->ts->pchan_is, lchan->ts->nr, lchan_nr);
+	rc = gsm_pchan2chan_nr(lchan->ts->pchan_is, lchan->ts->nr, lchan_nr);
+	/* Log an error so that we don't need to add logging to each caller of this function */
+	if (rc)
+		LOG_LCHAN(lchan, LOGL_ERROR,
+			  "Error encoding Channel Number: pchan %s ts %u ss %u%s\n",
+			  gsm_pchan_name(lchan->ts->pchan_from_config), lchan->ts->nr, lchan->nr,
+			  lchan->vamos.is_secondary ? " (VAMOS shadow)" : "");
+	return rc;
 }
 
 static const uint8_t subslots_per_pchan[] = {
@@ -677,23 +690,40 @@
 	}
 }
 
-void gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
-			   const struct gsm_lchan *lchan,
-			   uint8_t tsc)
+int gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
+			  const struct gsm_lchan *lchan,
+			  uint8_t tsc)
 {
-	cd->chan_nr = gsm_lchan2chan_nr(lchan);
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	if (chan_nr < 0) {
+		/* Log an error so that we don't need to add logging to each caller of this function */
+		LOG_LCHAN(lchan, LOGL_ERROR, "Error encoding Channel Number\n");
+		return chan_nr;
+	}
+	cd->chan_nr = chan_nr;
 	_chan_desc_fill_tail(cd, lchan, tsc);
+	return 0;
 }
 
 /* like gsm48_lchan2chan_desc() above, but use ts->pchan_from_config to
  * return a channel description based on what is configured, rather than
  * what the current state of the pchan type is */
-void gsm48_lchan2chan_desc_as_configured(struct gsm48_chan_desc *cd,
-					 const struct gsm_lchan *lchan,
-					 uint8_t tsc)
+int gsm48_lchan2chan_desc_as_configured(struct gsm48_chan_desc *cd,
+					const struct gsm_lchan *lchan,
+					uint8_t tsc)
 {
-	cd->chan_nr = gsm_pchan2chan_nr(lchan->ts->pchan_from_config, lchan->ts->nr, lchan->nr);
+	int chan_nr = gsm_pchan2chan_nr(lchan->ts->pchan_from_config, lchan->ts->nr, lchan->nr);
+	if (chan_nr < 0) {
+		/* Log an error so that we don't need to add logging to each caller of this function */
+		LOG_LCHAN(lchan, LOGL_ERROR,
+			  "Error encoding Channel Number: pchan %s ts %u ss %u%s (rc = %d)\n",
+			  gsm_pchan_name(lchan->ts->pchan_from_config), lchan->ts->nr, lchan->nr,
+			  lchan->vamos.is_secondary ? " (VAMOS shadow)" : "", chan_nr);
+		return chan_nr;
+	}
+	cd->chan_nr = chan_nr;
 	_chan_desc_fill_tail(cd, lchan, tsc);
+	return 0;
 }
 
 uint8_t gsm_ts_tsc(const struct gsm_bts_trx_ts *ts)
diff --git a/src/osmo-bsc/lcs_loc_req.c b/src/osmo-bsc/lcs_loc_req.c
index ee85c91..94c4e3b 100644
--- a/src/osmo-bsc/lcs_loc_req.c
+++ b/src/osmo-bsc/lcs_loc_req.c
@@ -369,7 +369,10 @@
 				.cause = BSSLAP_CAUSE_INTRA_BSS_HO,
 			},
 		};
-		gsm48_lchan2chan_desc(&apdu->reset.chan_desc, lchan, lchan->tsc);
+		if (gsm48_lchan2chan_desc(&apdu->reset.chan_desc, lchan, lchan->tsc)) {
+			lcs_loc_req_fail(LCS_CAUSE_SYSTEM_FAILURE, "Error encoding Channel Number");
+			return;
+		}
 	}
 
 	lcs_loc_req_send(lcs_loc_req, &bsslap);
diff --git a/src/osmo-bsc/osmo_bsc_lcls.c b/src/osmo-bsc/osmo_bsc_lcls.c
index 2c90d01..542bbf5 100644
--- a/src/osmo-bsc/osmo_bsc_lcls.c
+++ b/src/osmo-bsc/osmo_bsc_lcls.c
@@ -243,13 +243,19 @@
        uint32_t ip = enable ? conn->lcls.other->lchan->abis_ip.bound_ip : lchan->abis_ip.connect_ip;
        /* RSL_IE_IPAC_REMOTE_PORT */
        uint16_t port = enable ? conn->lcls.other->lchan->abis_ip.bound_port : lchan->abis_ip.connect_port;
+       struct msgb *msg;
 
        if (!conn->lcls.other) {
 	       LOGPFSM(conn->lcls.fi, "%s LCLS: other conn is not available!\n", enable ? "enable" : "disable");
                return;
        }
 
-       abis_rsl_sendmsg(rsl_make_ipacc_mdcx(lchan, ip, port));
+       msg = rsl_make_ipacc_mdcx(lchan, ip, port);
+       if (!msg) {
+	       LOGPFSML(conn->lcls.fi, LOGL_ERROR, "Error encoding IPACC MDCX\n");
+	       return;
+       }
+       abis_rsl_sendmsg(msg);
 }
 
 static inline bool lcls_check_toggle_allowed(const struct gsm_subscriber_connection *conn, bool enable)
diff --git a/src/osmo-bsc/system_information.c b/src/osmo-bsc/system_information.c
index 21cbcae..e8b3b7d 100644
--- a/src/osmo-bsc/system_information.c
+++ b/src/osmo-bsc/system_information.c
@@ -1018,7 +1018,8 @@
 		struct gsm48_chan_desc cd;
 
 		/* 10.5.2.5 (TV) CBCH Channel Description IE */
-		gsm48_lchan2chan_desc_as_configured(&cd, cbch_lchan, gsm_ts_tsc(cbch_lchan->ts));
+		if (gsm48_lchan2chan_desc_as_configured(&cd, cbch_lchan, gsm_ts_tsc(cbch_lchan->ts)))
+			return -EINVAL;
 		tail = tv_fixed_put(tail, GSM48_IE_CBCH_CHAN_DESC,
 				    sizeof(cd), (uint8_t *) &cd);
 		l2_plen += 1 + sizeof(cd);
diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c
index e653920..b741e25 100644
--- a/tests/handover/handover_test.c
+++ b/tests/handover/handover_test.c
@@ -100,10 +100,11 @@
 {
 	struct msgb *msg = msgb_alloc_headroom(256, 64, "RSL");
 	struct abis_rsl_dchan_hdr *dh;
-	uint8_t chan_nr = gsm_lchan2chan_nr(lchan);
 	uint8_t ulm[3], l1i[2], *buf;
 	struct gsm48_hdr *gh;
 	struct gsm48_meas_res *mr;
+	int chan_nr = gsm_lchan2chan_nr(lchan);
+	OSMO_ASSERT(chan_nr >= 0);
 
 	dh = (struct abis_rsl_dchan_hdr *) msgb_put(msg, sizeof(*dh));
 	dh->c.msg_discr = ABIS_RSL_MDISC_DED_CHAN;

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/24524
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I71ed6437c403a3f9336e17a94b4948fca295d853
Gerrit-Change-Number: 24524
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210602/028ca9bf/attachment.htm>


More information about the gerrit-log mailing list