<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/12643">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Work around bogus gcc-8.2 array-bounds warning/error<br><br>gcc-8.2 is printing the following warning, which is an error<br>when used -Werror like our --enable-werror:<br><br>In file included from gprs_bssgp.c:34:<br>In function ‘tl16v_put’,<br> inlined from ‘tvlv_put.part.3’ at ../../include/osmocom/gsm/tlv.h:156:9,<br> inlined from ‘tvlv_put’ at ../../include/osmocom/gsm/tlv.h:147:24,<br> inlined from ‘msgb_tvlv_push’ at ../../include/osmocom/gsm/tlv.h:386:2,<br> inlined from ‘bssgp_tx_dl_ud’ at gprs_bssgp.c:1162:4:<br>../../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]<br> memcpy(buf, val, len);<br><br>Where "130" seems to be the maximum value of uint8_t, shifted right one +<br>2. But even as we use strnlen() with "16" as maximum upper bound, gcc<br>still believes there's a way that the return value of gsm48_generate_mid_from_imsi()<br>could be 130. In fact, even the newly-added OSMO_ASSERT() inside<br>gsm48_generate_mid() doesn't help and gcc still insists there is a problem :(<br><br>Change-Id: I0a06daa19b7b5b5badbb8b3d81a54c45b88a60ec<br>---<br>M src/gb/gprs_bssgp.c<br>M src/gb/gprs_bssgp_bss.c<br>2 files changed, 21 insertions(+), 1 deletion(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c</span><br><span>index be7ef9f..4a4bab3 100644</span><br><span>--- a/src/gb/gprs_bssgp.c</span><br><span>+++ b/src/gb/gprs_bssgp.c</span><br><span>@@ -1157,10 +1157,17 @@</span><br><span> /* IMSI */</span><br><span> if (dup->imsi && strlen(dup->imsi)) {</span><br><span> uint8_t mi[GSM48_MID_MAX_SIZE];</span><br><span style="color: hsl(120, 100%, 40%);">+/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11,</span><br><span style="color: hsl(120, 100%, 40%);">+ * but somehow gcc (8.2) is not smart enough to figure this out and claims that</span><br><span style="color: hsl(120, 100%, 40%);">+ * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to</span><br><span style="color: hsl(120, 100%, 40%);">+ * mi[131], which is wrong */</span><br><span style="color: hsl(120, 100%, 40%);">+#pragma GCC diagnostic push</span><br><span style="color: hsl(120, 100%, 40%);">+#pragma GCC diagnostic ignored "-Warray-bounds"</span><br><span> int imsi_len = gsm48_generate_mid_from_imsi(mi, dup->imsi);</span><br><span> if (imsi_len > 2)</span><br><span> msgb_tvlv_push(msg, BSSGP_IE_IMSI,</span><br><span> imsi_len-2, mi+2);</span><br><span style="color: hsl(120, 100%, 40%);">+#pragma GCC diagnostic pop</span><br><span> }</span><br><span> </span><br><span> /* DRX parameters */</span><br><span>@@ -1220,7 +1227,14 @@</span><br><span> else</span><br><span> bgph->pdu_type = BSSGP_PDUT_PAGING_CS;</span><br><span> /* IMSI */</span><br><span style="color: hsl(120, 100%, 40%);">+/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11,</span><br><span style="color: hsl(120, 100%, 40%);">+ * but somehow gcc (8.2) is not smart enough to figure this out and claims that</span><br><span style="color: hsl(120, 100%, 40%);">+ * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to</span><br><span style="color: hsl(120, 100%, 40%);">+ * mi[131], which is wrong */</span><br><span style="color: hsl(120, 100%, 40%);">+#pragma GCC diagnostic push</span><br><span style="color: hsl(120, 100%, 40%);">+#pragma GCC diagnostic ignored "-Warray-bounds"</span><br><span> msgb_tvlv_put(msg, BSSGP_IE_IMSI, imsi_len-2, mi+2);</span><br><span style="color: hsl(120, 100%, 40%);">+#pragma GCC diagnostic pop</span><br><span> /* DRX Parameters */</span><br><span> msgb_tvlv_put(msg, BSSGP_IE_DRX_PARAMS, 2,</span><br><span> (uint8_t *) &drx_params);</span><br><span>diff --git a/src/gb/gprs_bssgp_bss.c b/src/gb/gprs_bssgp_bss.c</span><br><span>index 77350e2..bef9bb1 100644</span><br><span>--- a/src/gb/gprs_bssgp_bss.c</span><br><span>+++ b/src/gb/gprs_bssgp_bss.c</span><br><span>@@ -183,10 +183,16 @@</span><br><span> </span><br><span> if (!msg)</span><br><span> return -ENOMEM;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(120, 100%, 40%);">+/* gsm48_generate_mid_from_imsi() is guaranteed to never return more than 11,</span><br><span style="color: hsl(120, 100%, 40%);">+ * but somehow gcc (8.2) is not smart enough to figure this out and claims that</span><br><span style="color: hsl(120, 100%, 40%);">+ * the memcpy in msgb_tvlv_put() below will cause and out-of-bounds access up to</span><br><span style="color: hsl(120, 100%, 40%);">+ * mi[131], which is wrong */</span><br><span style="color: hsl(120, 100%, 40%);">+#pragma GCC diagnostic push</span><br><span style="color: hsl(120, 100%, 40%);">+#pragma GCC diagnostic ignored "-Warray-bounds"</span><br><span> /* strip the MI type and length values (2 bytes) */</span><br><span> if (imsi_len > 2)</span><br><span> msgb_tvlv_put(msg, BSSGP_IE_IMSI, imsi_len-2, mi+2);</span><br><span style="color: hsl(120, 100%, 40%);">+#pragma GCC diagnostic pop</span><br><span> LOGPC(DBSSGP, LOGL_NOTICE, "IMSI=%s ", imsi);</span><br><span> </span><br><span> return common_tx_radio_status2(msg, cause);</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/12643">change 12643</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/12643"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I0a06daa19b7b5b5badbb8b3d81a54c45b88a60ec </div>
<div style="display:none"> Gerrit-Change-Number: 12643 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>