[PATCH] osmo-pcu[master]: Fix CSN1 decoding: CSN_LEFT_ALIGNED_VAR_BMP bounds

[Neels Hofmeyr]

Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/805

to look at the new patch set (#2).

Fix CSN1 decoding: CSN_LEFT_ALIGNED_VAR_BMP bounds

Fix attempted read past vector boundaries in case of a starting bit offset !=
0, so that the last amount of bits read should be < 8. In the case of
CSN_LEFT_ALIGNED_VAR_BMP, the mod-8 calculation was flawed, and in the final
step, 8 bits were read instead of 6. This lead to -EINVAL being returned by
bitvec_get_bit_pos() and bogus resulting data.

Instead, read 8 bits only as long as at least 8 bits remain, and read any
remaining bits < 8 in a final step. Drop unneeded nB1 variable and an obvious
comment.

Adjust the unit test assertion in testCsnLeftAlignedVarBmpBounds() in
RLCMACTest.cpp.

Based on a fix by Aravind Sirsikar <Arvind.Sirsikar at radisys.com>, but
implemented differently.

Related: OS#1805
Change-Id: I490498c8da6b531f54acb673379379f7b10907c0
---
M src/csn1.cpp
M tests/rlcmac/RLCMACTest.cpp
2 files changed, 6 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/05/805/2

diff --git a/src/csn1.cpp b/src/csn1.cpp
index d51fe83..a1698a5 100644
--- a/src/csn1.cpp
+++ b/src/csn1.cpp
@@ -1110,22 +1110,21 @@
 
           { /* extract bits */
             guint8* pui8 = pui8DATA(data, pDescr->offset);
-            gint16 nB1  = no_of_bits & 0x07;/* no_of_bits Mod 8 */
 
-            while (no_of_bits > 0)
+            while (no_of_bits >= 8)
             {
               *pui8 = bitvec_read_field(vector, readIndex, 8);
               LOGPC(DCSN1, LOGL_NOTICE, "%s = %u | ", pDescr->sz , (unsigned)*pui8);
               pui8++;
               no_of_bits -= 8;
             }
-            if (nB1 > 0)
+            if (no_of_bits > 0)
             { 
-              *pui8 = bitvec_read_field(vector, readIndex, nB1);
+              *pui8 = bitvec_read_field(vector, readIndex, no_of_bits);
               LOGPC(DCSN1, LOGL_NOTICE, "%s = %u | ", pDescr->sz , (unsigned)*pui8);
               pui8++;
-              no_of_bits  -= nB1;
-              bit_offset += nB1; /* (nB1 is no_of_bits Mod 8) */
+              bit_offset += no_of_bits;
+              no_of_bits = 0;
             }
           }
         }
diff --git a/tests/rlcmac/RLCMACTest.cpp b/tests/rlcmac/RLCMACTest.cpp
index f451dbb..d1d4a6e 100644
--- a/tests/rlcmac/RLCMACTest.cpp
+++ b/tests/rlcmac/RLCMACTest.cpp
@@ -221,7 +221,6 @@
 
 	EGPRS_AckNack_Desc_t *urbb =
 		&data.u.Egprs_Packet_Downlink_Ack_Nack.EGPRS_AckNack.Desc;
-	memset(urbb->URBB, -1, sizeof(urbb->URBB));
 	decode_gsm_rlcmac_uplink(vector, &data);
 
 	/*
@@ -230,7 +229,7 @@
 	 * see data coming from bitvec_get_bit_pos() returning -EINVAL.
 	 */
 	OSMO_ASSERT(!strcmp(osmo_hexdump(urbb->URBB, 13),
-			    "7f ff ff ee 00 00 00 00 00 00 00 00 ea "));
+			    "7f ff ff ee 00 00 00 00 00 00 00 00 00 "));
 }
 
 int main(int argc, char *argv[])

-- 
To view, visit https://gerrit.osmocom.org/805
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I490498c8da6b531f54acb673379379f7b10907c0
Gerrit-PatchSet: 2
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>