[PATCH 1/3] bssgp: Always expect dup != NULL in bssgp_tx_dl_ud (Coverity)

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Jacob Erlbeck jerlbeck at sysmocom.de
Tue Apr 7 15:52:43 UTC 2015


Currently the implementation of bssgp_tx_dl_ud conditionally adds
some optional IE if dup != NULL. Later on is dereferences dup to
access qos_profile and fc, but this without checking dup in advance.
This may lead to an segmentation violation fault.

This commit changes the value range of the function to only accept
dup != NULL. An assertion will fail otherwise. All other explicit
checks for non-NULL are removed.

Fixes: Coverity CID 1040673
Sponsored-by: On-Waves ehf
---
 src/gb/gprs_bssgp.c | 49 ++++++++++++++++++++++++-------------------------
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c
index 2f23290..a3fd6aa 100644
--- a/src/gb/gprs_bssgp.c
+++ b/src/gb/gprs_bssgp.c
@@ -1092,6 +1092,8 @@ int bssgp_tx_dl_ud(struct msgb *msg, uint16_t pdu_lifetime,
 	uint16_t _pdu_lifetime = htons(pdu_lifetime); /* centi-seconds */
 	uint16_t drx_params;
 
+	OSMO_ASSERT(dup != NULL);
+
 	/* Identifiers from UP: TLLI, BVCI, NSEI (all in msgb->cb) */
 	if (bvci <= BVCI_PTM ) {
 		LOGP(DBSSGP, LOGL_ERROR, "Cannot send DL-UD to BVCI %u\n",
@@ -1124,35 +1126,32 @@ int bssgp_tx_dl_ud(struct msgb *msg, uint16_t pdu_lifetime,
 
 	/* FIXME: optional elements: Alignment, UTRAN CCO, LSA, PFI */
 
-	if (dup) {
-		/* Old TLLI to help BSS map from old->new */
-		if (dup->tlli) {
-			uint32_t tlli = htonl(*dup->tlli);
-			msgb_tvlv_push(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &tlli);
-		}
-
-		/* IMSI */
-		if (dup->imsi && strlen(dup->imsi)) {
-			uint8_t mi[10];
-			int imsi_len = gsm48_generate_mid_from_imsi(mi, dup->imsi);
-			if (imsi_len > 2)
-				msgb_tvlv_push(msg, BSSGP_IE_IMSI,
-						imsi_len-2, mi+2);
-		}
+	/* Old TLLI to help BSS map from old->new */
+	if (dup->tlli) {
+		uint32_t tlli = htonl(*dup->tlli);
+		msgb_tvlv_push(msg, BSSGP_IE_TLLI, 4, (uint8_t *) &tlli);
+	}
 
-		/* DRX parameters */
-		drx_params = htons(dup->drx_parms);
-		msgb_tvlv_push(msg, BSSGP_IE_DRX_PARAMS, 2,
-				(uint8_t *) &drx_params);
+	/* IMSI */
+	if (dup->imsi && strlen(dup->imsi)) {
+		uint8_t mi[10];
+		int imsi_len = gsm48_generate_mid_from_imsi(mi, dup->imsi);
+		if (imsi_len > 2)
+			msgb_tvlv_push(msg, BSSGP_IE_IMSI,
+				imsi_len-2, mi+2);
+	}
 
-		/* FIXME: Priority */
+	/* DRX parameters */
+	drx_params = htons(dup->drx_parms);
+	msgb_tvlv_push(msg, BSSGP_IE_DRX_PARAMS, 2,
+		(uint8_t *) &drx_params);
 
-		/* MS Radio Access Capability */
-		if (dup->ms_ra_cap.len)
-			msgb_tvlv_push(msg, BSSGP_IE_MS_RADIO_ACCESS_CAP,
-					dup->ms_ra_cap.len, dup->ms_ra_cap.v);
+	/* FIXME: Priority */
 
-	}
+	/* MS Radio Access Capability */
+	if (dup->ms_ra_cap.len)
+		msgb_tvlv_push(msg, BSSGP_IE_MS_RADIO_ACCESS_CAP,
+			dup->ms_ra_cap.len, dup->ms_ra_cap.v);
 
 	/* prepend the pdu lifetime */
 	msgb_tvlv_push(msg, BSSGP_IE_PDU_LIFETIME, 2, (uint8_t *)&_pdu_lifetime);
-- 
1.9.1




More information about the OpenBSC mailing list