Change in libosmocore[master]: Work around bogus gcc-8.2 array-bounds warning/error

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/12643 )

Change subject: Work around bogus gcc-8.2 array-bounds warning/error
......................................................................

Work around bogus gcc-8.2 array-bounds warning/error

gcc-8.2 is printing the following warning, which is an error
when used -Werror like our --enable-werror:

In file included from gprs_bssgp.c:34:
In function ‘tl16v_put’,
    inlined from ‘tvlv_put.part.3’ at ../../include/osmocom/gsm/tlv.h:156:9,
    inlined from ‘tvlv_put’ at ../../include/osmocom/gsm/tlv.h:147:24,
    inlined from ‘msgb_tvlv_push’ at ../../include/osmocom/gsm/tlv.h:386:2,
    inlined from ‘bssgp_tx_dl_ud’ at gprs_bssgp.c:1162:4:
../../include/osmocom/gsm/tlv.h:131:2: error: ‘memcpy’ forming offset [12, 130] is out of the bounds [0, 11] of object ‘mi’ with type ‘uint8_t[11]’ {aka ‘unsigned char[11]’} [-Werror=array-bounds]
  memcpy(buf, val, len);

Where "130" seems to be the maximum value of uint8_t, shifted right one +
2.  But even as we use strnlen() with "16" as maximum upper bound, gcc
still believes there's a way that the return value of gsm48_generate_mid_from_imsi()
could be 130.  In fact, even the newly-added OSMO_ASSERT() inside
gsm48_generate_mid() doesn't help and gcc still insists there is a problem :(

Change-Id: I0a06daa19b7b5b5badbb8b3d81a54c45b88a60ec
---
M src/gb/gprs_bssgp.c
M src/gb/gprs_bssgp_bss.c
2 files changed, 21 insertions(+), 1 deletion(-)

Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved



diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c
index be7ef9f..4a4bab3 100644
--- a/src/gb/gprs_bssgp.c
+++ b/src/gb/gprs_bssgp.c
@@ -1157,10 +1157,17 @@
 	/* IMSI */
 	if (dup->imsi && strlen(dup->imsi)) {
 		uint8_t mi[GSM48_MID_MAX_SIZE];
+/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11,
+ * but somehow gcc (8.2) is not smart enough to figure this out and claims that
+ * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to
+ * mi[131], which is wrong */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
 		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);
+#pragma GCC diagnostic pop
 	}
 
 	/* DRX parameters */
@@ -1220,7 +1227,14 @@
 	else
 		bgph->pdu_type = BSSGP_PDUT_PAGING_CS;
 	/* IMSI */
+/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11,
+ * but somehow gcc (8.2) is not smart enough to figure this out and claims that
+ * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to
+ * mi[131], which is wrong */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
 	msgb_tvlv_put(msg, BSSGP_IE_IMSI, imsi_len-2, mi+2);
+#pragma GCC diagnostic pop
 	/* DRX Parameters */
 	msgb_tvlv_put(msg, BSSGP_IE_DRX_PARAMS, 2,
 			(uint8_t *) &drx_params);
diff --git a/src/gb/gprs_bssgp_bss.c b/src/gb/gprs_bssgp_bss.c
index 77350e2..bef9bb1 100644
--- a/src/gb/gprs_bssgp_bss.c
+++ b/src/gb/gprs_bssgp_bss.c
@@ -183,10 +183,16 @@
 
 	if (!msg)
 		return -ENOMEM;
-
+/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11,
+ * but somehow gcc (8.2) is not smart enough to figure this out and claims that
+ * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to
+ * mi[131], which is wrong */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
 	/* strip the MI type and length values (2 bytes) */
 	if (imsi_len > 2)
 		msgb_tvlv_put(msg, BSSGP_IE_IMSI, imsi_len-2, mi+2);
+#pragma GCC diagnostic pop
 	LOGPC(DBSSGP, LOGL_NOTICE, "IMSI=%s ", imsi);
 
 	return common_tx_radio_status2(msg, cause);

-- 
To view, visit https://gerrit.osmocom.org/12643
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0a06daa19b7b5b5badbb8b3d81a54c45b88a60ec
Gerrit-Change-Number: 12643
Gerrit-PatchSet: 3
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190122/177b9a9f/attachment.htm>