Add function to properly encode RAI

Add gsm48_encode_ra() which takes appropriate struct as [out] parameter
instead of generic buffer. Using uint8_t buffer instead of proper struct
type prooved to be error-prone - see Coverity CID57877, CID57876.

Old gsm48_construct_ra() is made into tiny wrapper around new
function. The test output is adjusted because of the change in function
return value which was constant and hence ignored anyway.

Related: OS#1640
Change-Id: I31f9605277f4945f207c2c44ff82e62399f8db74

3 files changed, 22 insertions, 26 deletions

diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c
index d27a94f7..72282636 100644
--- a/src/gb/gprs_bssgp.c
+++ b/src/gb/gprs_bssgp.c
@@ -161,7 +161,6 @@ int bssgp_tx_suspend_ack(uint16_t nsei, uint32_t tlli,
 	struct bssgp_normal_hdr *bgph =
 		(struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph));
 	uint32_t _tlli;
-	uint8_t ra[6];
 
 	msgb_nsei(msg) = nsei;
 	msgb_bvci(msg) = 0; /* Signalling */
@@ -169,8 +168,7 @@ int bssgp_tx_suspend_ack(uint16_t nsei, uint32_t tlli,
 
 	_tlli = osmo_htonl(tlli);
 	msgb_tvlv_put(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &_tlli);
-	gsm48_construct_ra(ra, ra_id);
-	msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra);
+	bssgp_msgb_ra_put(msg, ra_id);
 	msgb_tvlv_put(msg, BSSGP_IE_SUSPEND_REF_NR, 1, &suspend_ref);
 
 	return gprs_ns_sendmsg(bssgp_nsi, msg);
@@ -185,7 +183,6 @@ int bssgp_tx_suspend_nack(uint16_t nsei, uint32_t tlli,
 	struct bssgp_normal_hdr *bgph =
 		(struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph));
 	uint32_t _tlli;
-	uint8_t ra[6];
 
 	msgb_nsei(msg) = nsei;
 	msgb_bvci(msg) = 0; /* Signalling */
@@ -193,8 +190,8 @@ int bssgp_tx_suspend_nack(uint16_t nsei, uint32_t tlli,
 
 	_tlli = osmo_htonl(tlli);
 	msgb_tvlv_put(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &_tlli);
-	gsm48_construct_ra(ra, ra_id);
-	msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra);
+	bssgp_msgb_ra_put(msg, ra_id);
+
 	if (cause)
 		msgb_tvlv_put(msg, BSSGP_IE_CAUSE, 1, cause);
 
@@ -209,7 +206,6 @@ int bssgp_tx_resume_ack(uint16_t nsei, uint32_t tlli,
 	struct bssgp_normal_hdr *bgph =
 		(struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph));
 	uint32_t _tlli;
-	uint8_t ra[6];
 
 	msgb_nsei(msg) = nsei;
 	msgb_bvci(msg) = 0; /* Signalling */
@@ -217,8 +213,7 @@ int bssgp_tx_resume_ack(uint16_t nsei, uint32_t tlli,
 
 	_tlli = osmo_htonl(tlli);
 	msgb_tvlv_put(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &_tlli);
-	gsm48_construct_ra(ra, ra_id);
-	msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra);
+	bssgp_msgb_ra_put(msg, ra_id);
 
 	return gprs_ns_sendmsg(bssgp_nsi, msg);
 }
@@ -231,7 +226,6 @@ int bssgp_tx_resume_nack(uint16_t nsei, uint32_t tlli,
 	struct bssgp_normal_hdr *bgph =
 		(struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph));
 	uint32_t _tlli;
-	uint8_t ra[6];
 
 	msgb_nsei(msg) = nsei;
 	msgb_bvci(msg) = 0; /* Signalling */
@@ -239,8 +233,8 @@ int bssgp_tx_resume_nack(uint16_t nsei, uint32_t tlli,
 
 	_tlli = osmo_htonl(tlli);
 	msgb_tvlv_put(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &_tlli);
-	gsm48_construct_ra(ra, ra_id);
-	msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra);
+	bssgp_msgb_ra_put(msg, ra_id);
+
 	if (cause)
 		msgb_tvlv_put(msg, BSSGP_IE_CAUSE, 1, cause);
 
@@ -259,7 +253,7 @@ int bssgp_create_cell_id(uint8_t *buf, const struct gprs_ra_id *raid,
 			 uint16_t cid)
 {
 	/* 6 octets RAC */
-	gsm48_construct_ra(buf, raid);
+	gsm48_encode_ra((struct gsm48_ra_id *)buf, raid);
 	/* 2 octets CID */
 	osmo_store16be(cid, buf+6);
 
@@ -1215,7 +1209,7 @@ int bssgp_tx_paging(uint16_t nsei, uint16_t ns_bvci,
 	uint16_t drx_params = osmo_htons(pinfo->drx_params);
 	uint8_t mi[10];
 	int imsi_len = gsm48_generate_mid_from_imsi(mi, pinfo->imsi);
-	uint8_t ra[6];
+	struct gsm48_ra_id ra;
 
 	if (imsi_len < 2)
 		return -EINVAL;
@@ -1241,12 +1235,11 @@ int bssgp_tx_paging(uint16_t nsei, uint16_t ns_bvci,
 		}
 		break;
 	case BSSGP_PAGING_LOCATION_AREA:
-		gsm48_construct_ra(ra, &pinfo->raid);
-		msgb_tvlv_put(msg, BSSGP_IE_LOCATION_AREA, 4, ra);
+		gsm48_encode_ra(&ra, &pinfo->raid);
+		msgb_tvlv_put(msg, BSSGP_IE_LOCATION_AREA, 4, (const uint8_t *)&ra);
 		break;
 	case BSSGP_PAGING_ROUTEING_AREA:
-		gsm48_construct_ra(ra, &pinfo->raid);
-		msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra);
+		bssgp_msgb_ra_put(msg, &pinfo->raid);
 		break;
 	case BSSGP_PAGING_BVCI:
 		{
diff --git a/src/gb/gprs_bssgp_bss.c b/src/gb/gprs_bssgp_bss.c
index 3939e258..c7e5e4d6 100644
--- a/src/gb/gprs_bssgp_bss.c
+++ b/src/gb/gprs_bssgp_bss.c
@@ -44,6 +44,14 @@ uint8_t *bssgp_msgb_tlli_put(struct msgb *msg, uint32_t tlli)
 	return msgb_tvlv_put(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &_tlli);
 }
 
+uint8_t *bssgp_msgb_ra_put(struct msgb *msg, const struct gprs_ra_id *ra_id)
+{
+	struct gsm48_ra_id ra;
+
+	gsm48_encode_ra(&ra, ra_id);
+	return msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, sizeof(ra), (const uint8_t *)&ra);
+}
+
 /*! GMM-SUSPEND.req (Chapter 10.3.6) */
 int bssgp_tx_suspend(uint16_t nsei, uint32_t tlli,
 		     const struct gprs_ra_id *ra_id)
@@ -51,7 +59,6 @@ int bssgp_tx_suspend(uint16_t nsei, uint32_t tlli,
 	struct msgb *msg = bssgp_msgb_alloc();
 	struct bssgp_normal_hdr *bgph =
 			(struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph));
-	uint8_t ra[6];
 
 	LOGP(DBSSGP, LOGL_NOTICE, "BSSGP (BVCI=0) Tx SUSPEND (TLLI=0x%04x)\n",
 		tlli);
@@ -60,9 +67,7 @@ int bssgp_tx_suspend(uint16_t nsei, uint32_t tlli,
 	bgph->pdu_type = BSSGP_PDUT_SUSPEND;
 
 	bssgp_msgb_tlli_put(msg, tlli);
-
-	gsm48_construct_ra(ra, ra_id);
-	msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra);
+	bssgp_msgb_ra_put(msg, ra_id);
 
 	return gprs_ns_sendmsg(bssgp_nsi, msg);
 }
@@ -74,7 +79,6 @@ int bssgp_tx_resume(uint16_t nsei, uint32_t tlli,
 	struct msgb *msg = bssgp_msgb_alloc();
 	struct bssgp_normal_hdr *bgph =
 			(struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph));
-	uint8_t ra[6];
 
 	LOGP(DBSSGP, LOGL_NOTICE, "BSSGP (BVCI=0) Tx RESUME (TLLI=0x%04x)\n",
 		tlli);
@@ -83,9 +87,7 @@ int bssgp_tx_resume(uint16_t nsei, uint32_t tlli,
 	bgph->pdu_type = BSSGP_PDUT_RESUME;
 
 	bssgp_msgb_tlli_put(msg, tlli);
-
-	gsm48_construct_ra(ra, ra_id);
-	msgb_tvlv_put(msg, BSSGP_IE_ROUTEING_AREA, 6, ra);
+	bssgp_msgb_ra_put(msg, ra_id);
 
 	msgb_tvlv_put(msg, BSSGP_IE_SUSPEND_REF_NR, 1, &suspend_ref);
 
diff --git a/src/gb/libosmogb.map b/src/gb/libosmogb.map
index 9a0dba5a..83a36213 100644
--- a/src/gb/libosmogb.map
+++ b/src/gb/libosmogb.map
@@ -9,6 +9,7 @@ bssgp_fc_ms_init;
 bssgp_msgb_alloc;
 bssgp_msgb_copy;
 bssgp_msgb_tlli_put;
+bssgp_msgb_ra_put;
 bssgp_parse_cell_id;
 bssgp_tx_bvc_block;
 bssgp_tx_bvc_reset;