From ebdc0d8c170ee2dbf23b19056d6c2d0ef316b3c2 Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Tue, 19 Oct 2021 16:48:16 +0200 Subject: [PATCH] csn1: Avoid failing if optional DownlinkDualCarrierCapability_r7 is missing All additional release fields are considered optional, and the CSN_DESCR for Content_t already marks almost all as such, except DownlinkDualCarrierCapability_r7. It has been found that some MS transmits a MS RA Capability with a Length=61 bits where the last bit in the buffer is setting the Exist bit for DownlinkDualCarrierCapability_r7 as 1. Hence, the CSN1 decoder failed to decode the whole message because it expected to keep reading there despite there's no more bytes to read. While this is could actually be considered an MS bug, let's relax our expectancies and simply consider the case { 1 } as it was { 0 }, and mark skip decoding DownlinkDualCarrierCapability_r7. That waht wireshark (packet-gsm_a_gsm.c) or pycrate do for instance. This patch itself doesn't fix the problem where actually the Exist bit is stored as 1 in the output decoded structure, but simply allows keep ongoing with decoding until the end. This issue will be fixed in a follow-up patch. Related: SYS#5552 Related: OS#4955 Related: OS#5020 Change-Id: I9a2541bd3544802a646890f32725201836abb0da --- src/csn1.h | 10 ++++++ src/csn1_dec.c | 62 +++++++++++++++++++++---------------- src/gsm_rlcmac.c | 2 +- tests/rlcmac/RLCMACTest.cpp | 8 ++--- tests/rlcmac/RLCMACTest.err | 2 +- tests/rlcmac/RLCMACTest.ok | 6 ++-- 6 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/csn1.h b/src/csn1.h index d470228d..285ae991 100644 --- a/src/csn1.h +++ b/src/csn1.h @@ -461,6 +461,16 @@ gint16 csnStreamEncoder(csnStream_t* ar, const CSN_DESCR* pDescr, struct bitvec #define M_TYPE(_STRUCT, _MEMBER, _MEMBER_TYPE)\ {CSN_TYPE, 0, {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), FALSE, #_MEMBER, 0, NULL} +/****************************************************************************** + * M_TYPE_OR_NULL(Par1, Par2, Par3) + * Similar to the M_TYPE except that not only the request set of bits but also the + * end of the message may be encountered when looking for the next element in + * the message. + * Covers the case {null | 0 | 1 < IE >} + *****************************************************************************/ +#define M_TYPE_OR_NULL(_STRUCT, _MEMBER, _MEMBER_TYPE)\ + {CSN_TYPE, 0, {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), TRUE, #_MEMBER, 0, NULL} + /****************************************************************************** * M_UNION(Par1, Par2) * Informs the CSN.1 library that a union follows and how many possible choices diff --git a/src/csn1_dec.c b/src/csn1_dec.c index 2d3f2421..4bc74f36 100644 --- a/src/csn1_dec.c +++ b/src/csn1_dec.c @@ -388,22 +388,26 @@ csnStreamDecoder(csnStream_t* ar, const CSN_DESCR* pDescr, struct bitvec *vector { gint16 Status; csnStream_t arT = *ar; - LOGPC(DCSN1, LOGL_DEBUG, " : %s | ", pDescr->sz); - csnStreamInit(&arT, bit_offset, remaining_bits_len); - Status = csnStreamDecoder(&arT, (const CSN_DESCR*)pDescr->descr.ptr, vector, readIndex, pvDATA(data, pDescr->offset)); - LOGPC(DCSN1, LOGL_DEBUG, ": End %s | ", pDescr->sz); - if (Status >= 0) + if (pDescr->may_be_null && remaining_bits_len == 0) { - remaining_bits_len = arT.remaining_bits_len; - bit_offset = arT.bit_offset; - pDescr++; + LOGPC(DCSN1, LOGL_DEBUG, " : %s = NULL | ", pDescr->sz); + } else { + LOGPC(DCSN1, LOGL_DEBUG, " : %s | ", pDescr->sz); + csnStreamInit(&arT, bit_offset, remaining_bits_len); + Status = csnStreamDecoder(&arT, (const CSN_DESCR*)pDescr->descr.ptr, vector, readIndex, pvDATA(data, pDescr->offset)); + LOGPC(DCSN1, LOGL_DEBUG, ": End %s | ", pDescr->sz); + if (Status >= 0) + { + remaining_bits_len = arT.remaining_bits_len; + bit_offset = arT.bit_offset; + } + else + { + /* Has already been processed: ProcessError("csnStreamDecoder", Status, pDescr); */ + return Status; + } } - else - { - /* Has already been processed: ProcessError("csnStreamDecoder", Status, pDescr); */ - return Status; - } - + pDescr++; break; } @@ -838,21 +842,25 @@ csnStreamDecoder(csnStream_t* ar, const CSN_DESCR* pDescr, struct bitvec *vector { gint16 Status; csnStream_t arT = *ar; - LOGPC(DCSN1, LOGL_DEBUG, " : %s | ", pDescr->sz); - csnStreamInit(&arT, bit_offset, remaining_bits_len); - Status = csnStreamDecoder(&arT, (const CSN_DESCR*)pDescr->descr.ptr, vector, readIndex, pvDATA(data, pDescr->offset)); - LOGPC(DCSN1, LOGL_DEBUG, " : End %s | ", pDescr->sz); - if (Status >= 0) + if (pDescr->may_be_null && remaining_bits_len == 0) { - remaining_bits_len = arT.remaining_bits_len; - bit_offset = arT.bit_offset; - pDescr++; + LOGPC(DCSN1, LOGL_DEBUG, " : %s = NULL | ", pDescr->sz); + } else { + LOGPC(DCSN1, LOGL_DEBUG, " : %s | ", pDescr->sz); + csnStreamInit(&arT, bit_offset, remaining_bits_len); + Status = csnStreamDecoder(&arT, (const CSN_DESCR*)pDescr->descr.ptr, vector, readIndex, pvDATA(data, pDescr->offset)); + LOGPC(DCSN1, LOGL_DEBUG, " : End %s | ", pDescr->sz); + if (Status >= 0) + { + remaining_bits_len = arT.remaining_bits_len; + bit_offset = arT.bit_offset; + } + else + { /* return error code Has already been processed: */ + return Status; + } } - else - { /* return error code Has already been processed: */ - return Status; - } - + pDescr++; break; } diff --git a/src/gsm_rlcmac.c b/src/gsm_rlcmac.c index f676645a..6793602c 100644 --- a/src/gsm_rlcmac.c +++ b/src/gsm_rlcmac.c @@ -928,7 +928,7 @@ CSN_DESCR_BEGIN (Content_t) /* additions in release 7 */ M_UINT_OR_NULL (Content_t, DTM_Handover_Capability, 1), M_NEXT_EXIST_OR_NULL(Content_t, Exist_DownlinkDualCarrierCapability_r7, 1), - M_TYPE (Content_t, DownlinkDualCarrierCapability_r7, DownlinkDualCarrierCapability_r7_t), + M_TYPE_OR_NULL (Content_t, DownlinkDualCarrierCapability_r7, DownlinkDualCarrierCapability_r7_t), M_UINT_OR_NULL (Content_t, FlexibleTimeslotAssignment, 1), M_UINT_OR_NULL (Content_t, GAN_PS_HandoverCapability, 1), diff --git a/tests/rlcmac/RLCMACTest.cpp b/tests/rlcmac/RLCMACTest.cpp index 4875bce6..88847de6 100644 --- a/tests/rlcmac/RLCMACTest.cpp +++ b/tests/rlcmac/RLCMACTest.cpp @@ -806,14 +806,14 @@ MS Radio Access Capability printf("=== Test decoding of MS RA Capability 4===\n"); rc = decode_gsm_ra_cap(bv_dec, &data); printf("decode_gsm_ra_cap() returns %d\n", rc); - OSMO_ASSERT(rc == -5); /* FIXME: should be 0 */ + OSMO_ASSERT(rc == 0); /* Make sure there's 3 values */ - OSMO_ASSERT(data.Count_MS_RA_capability_value == 0); /* FIXME: should be 3 */ + OSMO_ASSERT(data.Count_MS_RA_capability_value == 3); /* Make sure GPRS / EGPRS multislot class is parsed correctly */ - printf("GPRS multislot class = %u\n", get_ms_class_by_capability(&data)); /* FIXME: should be 12 */ - printf("EGPRS multislot class = %u\n", get_egprs_ms_class_by_capability(&data)); /* FIXME: should be 12 */ + printf("GPRS multislot class = %u\n", get_ms_class_by_capability(&data)); + printf("EGPRS multislot class = %u\n", get_egprs_ms_class_by_capability(&data)); bitvec_free(bv_dec); } diff --git a/tests/rlcmac/RLCMACTest.err b/tests/rlcmac/RLCMACTest.err index 045ba786..33a47c31 100644 --- a/tests/rlcmac/RLCMACTest.err +++ b/tests/rlcmac/RLCMACTest.err @@ -58,4 +58,4 @@ DCSN1 DEBUG csnStreamDecoder (EGPRS Packet Channel Request): Choice EGPRS_Packet DCSN1 DEBUG csnStreamDecoder (EGPRS Packet Channel Request): Choice EGPRS_PacketChannelRequest_Choice = 51 | : Content | RandomBits = 17 | : End Content | DCSN1 DEBUG csnStreamDecoder (EGPRS Packet Channel Request): Choice EGPRS_PacketChannelRequest_Choice = 55 | : Content | RandomBits = 25 | : End Content | DCSN1 DEBUG csnStreamDecoder (EGPRS Packet Channel Request): DCSN1 ERROR csnStreamDecoder: error STREAM_NOT_SUPPORTED (-8) at EGPRS_PacketChannelRequest_Choice (idx 0) -DCSN1 INFO csnStreamDecoder (RAcap): MS_RA_capability_value { | Choice MS_RA_capability_value_Choice = 1 | u.Content length = 61 | offset = 1 | RF_Power_Capability = 4 | Exist_A5_bits = 1 | A5_bits = 80 | ES_IND = 1 | PS = 1 | VGCS = 0 | VBS = 0 | Exist_Multislot_capability = 1 | : Multislot_capability | Exist_HSCSD_multislot_class = 0 | Exist_GPRS_multislot_class = 1 | GPRS_multislot_class = 12 | GPRS_Extended_Dynamic_Allocation_Capability = 1 | Exist_SM = 0 | Exist_ECSD_multislot_class = 0 | Exist_EGPRS_multislot_class = 1 | EGPRS_multislot_class = 12 | EGPRS_Extended_Dynamic_Allocation_Capability = 1 | Exist_DTM_GPRS_multislot_class = 0 | : End Multislot_capability | Exist_Eight_PSK_Power_Capability = 1 | Eight_PSK_Power_Capability = 2 | COMPACT_Interference_Measurement_Capability = 0 | Revision_Level_Indicator = 1 | UMTS_FDD_Radio_Access_Technology_Capability = 1 | UMTS_384_TDD_Radio_Access_Technology_Capability = 0 | CDMA2000_Radio_Access_Technology_Capability = 0 | UMTS_128_TDD_Radio_Access_Technology_Capability = 0 | GERAN_Feature_Package_1 = 1 | Exist_Extended_DTM_multislot_class = 0 | Modulation_based_multislot_class_support = 0 | Exist_HighMultislotCapability = 0 | Exist_GERAN_lu_ModeCapability = 0 | GMSK_MultislotPowerProfile = 0 | EightPSK_MultislotProfile = 0 | MultipleTBF_Capability = 0 | DownlinkAdvancedReceiverPerformance = 0 | ExtendedRLC_MAC_ControlMessageSegmentionsCapability = 0 | DTM_EnhancementsCapability = 0 | Exist_DTM_GPRS_HighMultislotClass = 0 | PS_HandoverCapability = 0 | DTM_Handover_Capability = 0 | Exist_DownlinkDualCarrierCapability_r7 = 1 | : DownlinkDualCarrierCapability_r7 | DCSN1 ERROR csnStreamDecoder: error NEED_MORE BITS TO UNPACK (-5) at MultislotCapabilityReductionForDL_DualCarrier (idx 72): End DownlinkDualCarrierCapability_r7 | +DCSN1 INFO csnStreamDecoder (RAcap): MS_RA_capability_value { | Choice MS_RA_capability_value_Choice = 1 | u.Content length = 61 | offset = 1 | RF_Power_Capability = 4 | Exist_A5_bits = 1 | A5_bits = 80 | ES_IND = 1 | PS = 1 | VGCS = 0 | VBS = 0 | Exist_Multislot_capability = 1 | : Multislot_capability | Exist_HSCSD_multislot_class = 0 | Exist_GPRS_multislot_class = 1 | GPRS_multislot_class = 12 | GPRS_Extended_Dynamic_Allocation_Capability = 1 | Exist_SM = 0 | Exist_ECSD_multislot_class = 0 | Exist_EGPRS_multislot_class = 1 | EGPRS_multislot_class = 12 | EGPRS_Extended_Dynamic_Allocation_Capability = 1 | Exist_DTM_GPRS_multislot_class = 0 | : End Multislot_capability | Exist_Eight_PSK_Power_Capability = 1 | Eight_PSK_Power_Capability = 2 | COMPACT_Interference_Measurement_Capability = 0 | Revision_Level_Indicator = 1 | UMTS_FDD_Radio_Access_Technology_Capability = 1 | UMTS_384_TDD_Radio_Access_Technology_Capability = 0 | CDMA2000_Radio_Access_Technology_Capability = 0 | UMTS_128_TDD_Radio_Access_Technology_Capability = 0 | GERAN_Feature_Package_1 = 1 | Exist_Extended_DTM_multislot_class = 0 | Modulation_based_multislot_class_support = 0 | Exist_HighMultislotCapability = 0 | Exist_GERAN_lu_ModeCapability = 0 | GMSK_MultislotPowerProfile = 0 | EightPSK_MultislotProfile = 0 | MultipleTBF_Capability = 0 | DownlinkAdvancedReceiverPerformance = 0 | ExtendedRLC_MAC_ControlMessageSegmentionsCapability = 0 | DTM_EnhancementsCapability = 0 | Exist_DTM_GPRS_HighMultislotClass = 0 | PS_HandoverCapability = 0 | DTM_Handover_Capability = 0 | Exist_DownlinkDualCarrierCapability_r7 = 1 | : DownlinkDualCarrierCapability_r7 = NULL | FlexibleTimeslotAssignment = NULL | GAN_PS_HandoverCapability = NULL | RLC_Non_persistentMode = NULL | ReducedLatencyCapability = NULL | UplinkEGPRS2 = NULL | DownlinkEGPRS2 = NULL | EUTRA_FDD_Support = NULL | EUTRA_TDD_Support = NULL | GERAN_To_EUTRAN_supportInGERAN_PTM = NULL | PriorityBasedReselectionSupport = NULL | MS_RA_capability_value } | MS_RA_capability_value { | Choice MS_RA_capability_value_Choice = 3 | u.Content length = 36 | offset = 1 | RF_Power_Capability = 1 | Exist_A5_bits = 0 | ES_IND = 1 | PS = 1 | VGCS = 0 | VBS = 0 | Exist_Multislot_capability = 0 | Exist_Eight_PSK_Power_Capability = 1 | Eight_PSK_Power_Capability = 2 | COMPACT_Interference_Measurement_Capability = 0 | Revision_Level_Indicator = 1 | UMTS_FDD_Radio_Access_Technology_Capability = 1 | UMTS_384_TDD_Radio_Access_Technology_Capability = 0 | CDMA2000_Radio_Access_Technology_Capability = 0 | UMTS_128_TDD_Radio_Access_Technology_Capability = 0 | GERAN_Feature_Package_1 = 1 | Exist_Extended_DTM_multislot_class = 0 | Modulation_based_multislot_class_support = 0 | Exist_HighMultislotCapability = 0 | Exist_GERAN_lu_ModeCapability = 0 | GMSK_MultislotPowerProfile = 0 | EightPSK_MultislotProfile = 0 | MultipleTBF_Capability = 0 | DownlinkAdvancedReceiverPerformance = 0 | ExtendedRLC_MAC_ControlMessageSegmentionsCapability = 0 | DTM_EnhancementsCapability = 0 | Exist_DTM_GPRS_HighMultislotClass = 0 | PS_HandoverCapability = 0 | DTM_Handover_Capability = 0 | Exist_DownlinkDualCarrierCapability_r7 = 1 | : DownlinkDualCarrierCapability_r7 = NULL | FlexibleTimeslotAssignment = NULL | GAN_PS_HandoverCapability = NULL | RLC_Non_persistentMode = NULL | ReducedLatencyCapability = NULL | UplinkEGPRS2 = NULL | DownlinkEGPRS2 = NULL | EUTRA_FDD_Support = NULL | EUTRA_TDD_Support = NULL | GERAN_To_EUTRAN_supportInGERAN_PTM = NULL | PriorityBasedReselectionSupport = NULL | MS_RA_capability_value } | MS_RA_capability_value { | Choice MS_RA_capability_value_Choice = 7 | u.Content length = 36 | offset = 1 | RF_Power_Capability = 4 | Exist_A5_bits = 0 | ES_IND = 1 | PS = 1 | VGCS = 0 | VBS = 0 | Exist_Multislot_capability = 0 | Exist_Eight_PSK_Power_Capability = 1 | Eight_PSK_Power_Capability = 2 | COMPACT_Interference_Measurement_Capability = 0 | Revision_Level_Indicator = 1 | UMTS_FDD_Radio_Access_Technology_Capability = 1 | UMTS_384_TDD_Radio_Access_Technology_Capability = 0 | CDMA2000_Radio_Access_Technology_Capability = 0 | UMTS_128_TDD_Radio_Access_Technology_Capability = 0 | GERAN_Feature_Package_1 = 1 | Exist_Extended_DTM_multislot_class = 0 | Modulation_based_multislot_class_support = 0 | Exist_HighMultislotCapability = 0 | Exist_GERAN_lu_ModeCapability = 0 | GMSK_MultislotPowerProfile = 0 | EightPSK_MultislotProfile = 0 | MultipleTBF_Capability = 0 | DownlinkAdvancedReceiverPerformance = 0 | ExtendedRLC_MAC_ControlMessageSegmentionsCapability = 0 | DTM_EnhancementsCapability = 0 | Exist_DTM_GPRS_HighMultislotClass = 0 | PS_HandoverCapability = 0 | DTM_Handover_Capability = 0 | Exist_DownlinkDualCarrierCapability_r7 = 1 | : DownlinkDualCarrierCapability_r7 = NULL | FlexibleTimeslotAssignment = NULL | GAN_PS_HandoverCapability = NULL | RLC_Non_persistentMode = NULL | ReducedLatencyCapability = NULL | UplinkEGPRS2 = NULL | DownlinkEGPRS2 = NULL | EUTRA_FDD_Support = NULL | EUTRA_TDD_Support = NULL | GERAN_To_EUTRAN_supportInGERAN_PTM = NULL | PriorityBasedReselectionSupport = NULL | MS_RA_capability_value } | Padding = 0| diff --git a/tests/rlcmac/RLCMACTest.ok b/tests/rlcmac/RLCMACTest.ok index 27800b0d..86364c45 100644 --- a/tests/rlcmac/RLCMACTest.ok +++ b/tests/rlcmac/RLCMACTest.ok @@ -214,6 +214,6 @@ decode_egprs_pkt_ch_req(0x6f9) returns 0 decode_egprs_pkt_ch_req(0x7ea) returns -8 *** testRAcap4 *** === Test decoding of MS RA Capability 4=== -decode_gsm_ra_cap() returns -5 -GPRS multislot class = 0 -EGPRS multislot class = 0 +decode_gsm_ra_cap() returns 0 +GPRS multislot class = 12 +EGPRS multislot class = 12