summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorMax <msuraev@sysmocom.de>2018-01-05 14:19:33 +0100
committerMax <msuraev@sysmocom.de>2018-01-08 13:02:07 +0000
commitf1ad60e4d861d5ff462a8d7ab481ad082360f346 (patch)
treed3cbc64a221e5a163b6407aaf6f9a7c65d76472c /src
parente1a511b0319bc2d8fc271aaee52d3a8ce2acf1e1 (diff)
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
Diffstat (limited to 'src')
-rw-r--r--src/gb/gprs_bssgp.c29
-rw-r--r--src/gb/gprs_bssgp_bss.c18
-rw-r--r--src/gb/libosmogb.map1
-rw-r--r--src/gsm/gsm48.c41
-rw-r--r--src/gsm/libosmogsm.map1
5 files changed, 44 insertions, 46 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;
diff --git a/src/gsm/gsm48.c b/src/gsm/gsm48.c
index a7daea47..88760599 100644
--- a/src/gsm/gsm48.c
+++ b/src/gsm/gsm48.c
@@ -689,33 +689,34 @@ void gsm48_parse_ra(struct gprs_ra_id *raid, const uint8_t *buf)
raid->rac = buf[5];
}
-/*! Encode a TS 04.08 Routing Area Identifier
- * \param[out] buf Caller-provided output buffer of 6 bytes
+/*! Encode a 3GPP TS 24.008 ยง 10.5.5.15 Routing area identification
+ * \param[out] out Caller-provided packed struct
* \param[in] raid Routing Area ID to be encoded
- * \returns number of bytes used in \a buf */
-int gsm48_construct_ra(uint8_t *buf, const struct gprs_ra_id *raid)
+ */
+void gsm48_encode_ra(struct gsm48_ra_id *out, const struct gprs_ra_id *raid)
{
- uint16_t mcc = raid->mcc;
- uint16_t mnc = raid->mnc;
- uint16_t _lac;
+ out->lac = osmo_htons(raid->lac);
+ out->rac = raid->rac;
- buf[0] = ((mcc / 100) % 10) | (((mcc / 10) % 10) << 4);
- buf[1] = (mcc % 10);
+ out->digits[0] = ((raid->mcc / 100) % 10) | (((raid->mcc / 10) % 10) << 4);
+ out->digits[1] = raid->mcc % 10;
- /* I wonder who came up with the stupidity of encoding the MNC
- * differently depending on how many digits its decimal number has! */
- if (mnc < 100) {
- buf[1] |= 0xf0;
- buf[2] = ((mnc / 10) % 10) | ((mnc % 10) << 4);
+ if (raid->mnc < 100) {
+ out->digits[1] |= 0xf0;
+ out->digits[2] = ((raid->mnc / 10) % 10) | ((raid->mnc % 10) << 4);
} else {
- buf[1] |= (mnc % 10) << 4;
- buf[2] = ((mnc / 100) % 10) | (((mnc / 10) % 10) << 4);
+ out->digits[1] |= (raid->mnc % 10) << 4;
+ out->digits[2] = ((raid->mnc / 100) % 10) | (((raid->mnc / 10) % 10) << 4);
}
+}
- _lac = osmo_htons(raid->lac);
- memcpy(buf + 3, &_lac, 2);
-
- buf[5] = raid->rac;
+/*! Encode a TS 04.08 Routing Area Identifier
+ * \param[out] buf Caller-provided output buffer of 6 bytes
+ * \param[in] raid Routing Area ID to be encoded
+ * \returns number of bytes used in \a buf */
+int gsm48_construct_ra(uint8_t *buf, const struct gprs_ra_id *raid)
+{
+ gsm48_encode_ra((struct gsm48_ra_id *)buf, raid);
return 6;
}
diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map
index d915234c..5611ba3f 100644
--- a/src/gsm/libosmogsm.map
+++ b/src/gsm/libosmogsm.map
@@ -205,6 +205,7 @@ gsm48_cc_msg_name;
gsm48_rr_msg_name;
gsm48_cc_state_name;
gsm48_construct_ra;
+gsm48_encode_ra;
gsm48_hdr_gmm_cipherable;
gsm48_decode_bcd_number;
gsm48_decode_bearer_cap;