laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/32014 )

Change subject: Fix parsing of TLV_TYPE_SINGLE_TV
......................................................................

Fix parsing of TLV_TYPE_SINGLE_TV

The decoding path of TLV_TYPE_SINGLE_TV is wrong, since it is not
shifting right the tag before using it. On the other hand, the encoding
path (tlv_encode_one) is doing that, so it is clear there's a bug.

It seems that in order to workaround the bug some IEs in gsm_04_08.h (TS
24.008 and TS 44.018) were defined incorrectly (eg 0x80) while the spec
clearly assigns eg. "8" to it, and makes sure no full byte IEI collides.
Some other IEIs like GSM48_IE_GMM_CIPH_CKSN which are also of the same
type were already correctly defined as 0x08.

Change-Id: I799e35dc8d4d153fa63bf50563a5482cdf4de2d7
---
M TODO-RELEASE
M include/osmocom/gsm/protocol/gsm_04_08.h
M src/gsm/gsm48.c
M src/gsm/tlv_parser.c
4 files changed, 39 insertions(+), 12 deletions(-)

Approvals:
  Jenkins Builder: Verified
  fixeria: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved




diff --git a/TODO-RELEASE b/TODO-RELEASE
index 8ecd7a6..2935a81 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -7,4 +7,5 @@
 # If any interfaces have been added since the last public release: c:r:a + 1.
 # If any interfaces have been removed or changed since the last public 
release: c:r:0.
 #library       what                    description / commit summary line
-libosmogsm  new header osmocom/gsm/protocol/gsm_44_060.h
\ No newline at end of file
+libosmogsm  new header osmocom/gsm/protocol/gsm_44_060.h
+libosmocore ADD     new defines in osmocom/gsm/protocol/gsm_04_08.h (old ones 
marked deprecated)
\ No newline at end of file
diff --git a/include/osmocom/gsm/protocol/gsm_04_08.h 
b/include/osmocom/gsm/protocol/gsm_04_08.h
index 66aa135..7b8979b 100644
--- a/include/osmocom/gsm/protocol/gsm_04_08.h
+++ b/include/osmocom/gsm/protocol/gsm_04_08.h
@@ -1734,6 +1734,10 @@
 #define GSM48_IE_FRQSHORT_AFTER        0x02
 #define GSM48_IE_MUL_RATE_CFG  0x03    /* 10.5.2.21aa */
 #define GSM48_IE_FREQ_L_AFTER  0x05
+#define GSM48_IE_GROUP_CIP_SEQ_HO 0x08 /* HO = Half Octet Tag */
+#define GSM48_IE_CIP_MODE_SET_HO 0x09 /* HO = Half Octet Tag */
+#define GSM48_IE_GPRS_RESUMPT_HO 0xc0 /* HO = Half Octet Tag */
+#define GSM48_IE_SYNC_IND_HO   0x0d /* HO = Half Octet Tag */
 #define GSM48_IE_MSLOT_DESC    0x10
 #define GSM48_IE_CHANMODE_2    0x11
 #define GSM48_IE_FRQSHORT_BEFORE 0x12
@@ -1775,20 +1779,21 @@
 #define GSM48_IE_START_TIME    0x7c
 #define GSM48_IE_INDIVIDUAL_PRIORITIES 0x7c /* 44.018 Section 9.1.7 */
 #define GSM48_IE_TIMING_ADVANCE        0x7d
-#define GSM48_IE_GROUP_CIP_SEQ 0x80
-#define GSM48_IE_CIP_MODE_SET  0x90
-#define GSM48_IE_GPRS_RESUMPT  0xc0
-#define GSM48_IE_SYNC_IND      0xd0
+#define GSM48_IE_GROUP_CIP_SEQ 0x80 /* DEPRECATED, use 
GSM48_IE_GROUP_CIP_SEQ_HO instead */
+#define GSM48_IE_CIP_MODE_SET  0x90  /* DEPRECATED, use 
GSM48_IE_CIP_MODE_SET_HO instead */
+#define GSM48_IE_GPRS_RESUMPT  0xc0 /* DEPRECATED, use 
GSM48_IE_GPRS_RESUMPT_HO instead */
+#define GSM48_IE_SYNC_IND      0xd0 /* DEPRECATED, use GSM48_IE_SYNC_IND_HO 
instead */
 /* System Information 4 (types are equal IEs above) */
 #define GSM48_IE_CBCH_CHAN_DESC        0x64
 #define GSM48_IE_CBCH_MOB_AL   0x72

 /* Additional MM elements */
+#define GSM48_IE_PRIORITY_LEV_HO 0x08 /* HO = Half Octet Tag */
 #define GSM48_IE_LOCATION_AREA 0x13
 #define GSM48_IE_AUTN          0x20
 #define GSM48_IE_AUTH_RES_EXT  0x21
 #define GSM48_IE_AUTS          0x22
-#define GSM48_IE_PRIORITY_LEV  0x80
+#define GSM48_IE_PRIORITY_LEV  0x80 /* DEPRECATED, use 
GSM48_IE_PRIORITY_LEV_HO instead */
 #define GSM48_IE_FOLLOW_ON_PROC        0xa1
 #define GSM48_IE_CTS_PERMISSION        0xa2

diff --git a/src/gsm/gsm48.c b/src/gsm/gsm48.c
index 919500b..1657864 100644
--- a/src/gsm/gsm48.c
+++ b/src/gsm/gsm48.c
@@ -129,10 +129,10 @@
                [GSM48_IE_REALTIME_DIFF]        = { TLV_TYPE_TLV },
                [GSM48_IE_START_TIME]           = { TLV_TYPE_FIXED, 2 },
                [GSM48_IE_TIMING_ADVANCE]       = { TLV_TYPE_TV },
-               [GSM48_IE_GROUP_CIP_SEQ]        = { TLV_TYPE_SINGLE_TV },
-               [GSM48_IE_CIP_MODE_SET]         = { TLV_TYPE_SINGLE_TV },
-               [GSM48_IE_GPRS_RESUMPT]         = { TLV_TYPE_SINGLE_TV },
-               [GSM48_IE_SYNC_IND]             = { TLV_TYPE_SINGLE_TV },
+               [GSM48_IE_GROUP_CIP_SEQ_HO]     = { TLV_TYPE_SINGLE_TV },
+               [GSM48_IE_CIP_MODE_SET_HO]      = { TLV_TYPE_SINGLE_TV },
+               [GSM48_IE_GPRS_RESUMPT_HO]      = { TLV_TYPE_SINGLE_TV },
+               [GSM48_IE_SYNC_IND_HO]          = { TLV_TYPE_SINGLE_TV },
        },
 };

@@ -148,7 +148,7 @@
                [GSM48_IE_NET_DST]              = { TLV_TYPE_TLV },

                [GSM48_IE_LOCATION_AREA]        = { TLV_TYPE_FIXED, 5 },
-               [GSM48_IE_PRIORITY_LEV]         = { TLV_TYPE_SINGLE_TV },
+               [GSM48_IE_PRIORITY_LEV_HO]      = { TLV_TYPE_SINGLE_TV },
                [GSM48_IE_FOLLOW_ON_PROC]       = { TLV_TYPE_T },
                [GSM48_IE_CTS_PERMISSION]       = { TLV_TYPE_T },
        },
diff --git a/src/gsm/tlv_parser.c b/src/gsm/tlv_parser.c
index de76688..e47b94f 100644
--- a/src/gsm/tlv_parser.c
+++ b/src/gsm/tlv_parser.c
@@ -241,7 +241,9 @@
        *o_tag = tag;

        /* single octet TV IE */
-       if (def->def[tag & 0xf0].type == TLV_TYPE_SINGLE_TV) {
+       if (def->def[tag >> 4].type == TLV_TYPE_SINGLE_TV
+           /* backward compat for old IEs with half-octet tag defined as 0xN0: 
*/
+           || ((tag > 0x0f) && (def->def[tag & 0xf0].type == 
TLV_TYPE_SINGLE_TV))) {
                *o_tag = tag & 0xf0;
                *o_val = buf;
                *o_len = 1;

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I799e35dc8d4d153fa63bf50563a5482cdf4de2d7
Gerrit-Change-Number: 32014
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-MessageType: merged

Reply via email to