<p>fixeria <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/26308">View Change</a></p><div style="white-space:pre-wrap">Approvals:
laforge: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bitvec_read_field(): fix incorrect bit-shift issue found by UBSan<br><br>While running a sanitized version of the bitvec_test I get:<br><br> bitvec.c:492:24: runtime error: shift exponent 64 is too large<br> for 64-bit type 'long unsigned int'<br><br>This error is triggered by the following line in the bitvec_test:<br><br> _bitvec_read_field(0, 8 * 8 + 1); /* too many bits */<br><br>which basically tries to parse more bits (65) than the test vector<br>actually has (64). The problem is that we don't check if the<br>given vector has enough data *before* entering the parsing loop,<br>so we end up doing weird bit-shifts and getting weird values:<br><br> bitvec_read_field(idx=0, len=65) => bd5b7ddffdd7b5db (error)<br><br>Unfortunately, this problem remained unnoticed so far because in<br>'tests/testsuite.at' we don't check if stderr is empty. This is<br>fixed in a follow up change [1].<br><br>Rather than checking for errors in every loop iteration, do this<br>once and return early if the overrun is possible with the given<br>offset and length arguments.<br><br>Change-Id: I4deeabba7ebb720cdbe7c85b37bc011d05bdfa65<br>Related: [1] Ia82b92eddb18dc596881abcef2f098dc7385538b<br>---<br>M src/bitvec.c<br>M tests/bitvec/bitvec_test.ok<br>2 files changed, 8 insertions(+), 5 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/bitvec.c b/src/bitvec.c</span><br><span>index b411a72..2303a0d 100644</span><br><span>--- a/src/bitvec.c</span><br><span>+++ b/src/bitvec.c</span><br><span>@@ -480,15 +480,18 @@</span><br><span> {</span><br><span> unsigned int i;</span><br><span> uint64_t ui = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Prevent bitvec overrun due to incorrect index and/or length */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (len && bytenum_from_bitnum(*read_index + len - 1) >= bv->data_len) {</span><br><span style="color: hsl(120, 100%, 40%);">+ errno = EOVERFLOW;</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> bv->cur_bit = *read_index;</span><br><span> errno = 0;</span><br><span> </span><br><span> for (i = 0; i < len; i++) {</span><br><span> int bit = bitvec_get_bit_pos((const struct bitvec *)bv, bv->cur_bit);</span><br><span style="color: hsl(0, 100%, 40%);">- if (bit < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- errno = -bit;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span> if (bit)</span><br><span> ui |= ((uint64_t)1 << (len - i - 1));</span><br><span> bv->cur_bit++;</span><br><span>diff --git a/tests/bitvec/bitvec_test.ok b/tests/bitvec/bitvec_test.ok</span><br><span>index a0e31d3..d87ac7e 100644</span><br><span>--- a/tests/bitvec/bitvec_test.ok</span><br><span>+++ b/tests/bitvec/bitvec_test.ok</span><br><span>@@ -185,7 +185,7 @@</span><br><span> bitvec_read_field(idx=10, len=3) => 5 (success)</span><br><span> bitvec_read_field(idx=10, len=1) => 1 (success)</span><br><span> bitvec_read_field(idx=512, len=16) => 0 (error)</span><br><span style="color: hsl(0, 100%, 40%);">-bitvec_read_field(idx=0, len=65) => bd5b7ddffdd7b5db (error)</span><br><span style="color: hsl(120, 100%, 40%);">+bitvec_read_field(idx=0, len=65) => 0 (error)</span><br><span> bitvec_read_field(idx=64, len=16) => 0 (error)</span><br><span> </span><br><span> bitvec ok.</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/26308">change 26308</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/c/libosmocore/+/26308"/><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-Change-Id: I4deeabba7ebb720cdbe7c85b37bc011d05bdfa65 </div>
<div style="display:none"> Gerrit-Change-Number: 26308 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>