aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPau Espin Pedrol <pespin@sysmocom.de>2021-10-20 16:50:29 +0200
committerPau Espin Pedrol <pespin@sysmocom.de>2021-10-20 18:50:49 +0200
commit6ba9c7b91899700b7e34bac97b6f1b848a541c27 (patch)
treeaacb7c8da8ea50192c3e6504c05d15e1bce58bfe
parent4d0537162b14394f9a66f42002a80d0f7c354f25 (diff)
csn1: Avoid failing if optional DownlinkDualCarrierCapability_r7 is missing
All additional release fields in RadioAccesCapabilities 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 <end> } as it was { 0 }, and mark skip decoding DownlinkDualCarrierCapability_r7. That what 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. This patch is a port from patch fixing same issue in the osmo-pcu.git copy of csn1 decoder: https://git.osmocom.org/osmo-pcu/commit/?id=ebdc0d8c170ee2dbf23b19056d6c2d0ef316b3c2
-rw-r--r--epan/dissectors/packet-csn1.c66
-rw-r--r--epan/dissectors/packet-csn1.h11
-rw-r--r--epan/dissectors/packet-gsm_rlcmac.c2
3 files changed, 47 insertions, 32 deletions
diff --git a/epan/dissectors/packet-csn1.c b/epan/dissectors/packet-csn1.c
index 447b6e35cf..b54b672a5f 100644
--- a/epan/dissectors/packet-csn1.c
+++ b/epan/dissectors/packet-csn1.c
@@ -531,25 +531,26 @@ csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pDescr, t
csnStream_t arT = *ar;
proto_item *ti;
proto_tree *test_tree;
-
- test_tree = proto_tree_add_subtree_format(tree, tvb, bit_offset>>3, 1, ett_csn1, &ti, "%s", pDescr->sz);
-
- csnStreamInit(&arT, bit_offset, remaining_bits_len, ar->pinfo);
- Status = csnStreamDissector(test_tree, &arT, (const CSN_DESCR*)pDescr->descr.ptr, tvb, pvDATA(data, pDescr->offset), ett_csn1);
-
- if (Status >= 0)
- {
- proto_item_set_len(ti,((arT.bit_offset-1)>>3) - (bit_offset>>3)+1);
- remaining_bits_len = arT.remaining_bits_len;
- bit_offset = arT.bit_offset;
- pDescr++;
- }
- else
+ if (pDescr->may_be_null && remaining_bits_len == 0)
{
- /* Has already been processed: ProcessError("csnStreamDissector", Status, pDescr); */
- return Status;
+ proto_tree_add_none_format(tree, hf_null_data, tvb, 0, 0, "[NULL data]: %s Not Present", pDescr->sz);
+ } else {
+ test_tree = proto_tree_add_subtree_format(tree, tvb, bit_offset>>3, 1, ett_csn1, &ti, "%s", pDescr->sz);
+ csnStreamInit(&arT, bit_offset, remaining_bits_len, ar->pinfo);
+ Status = csnStreamDissector(test_tree, &arT, (const CSN_DESCR*)pDescr->descr.ptr, tvb, pvDATA(data, pDescr->offset), ett_csn1);
+ if (Status >= 0)
+ {
+ proto_item_set_len(ti,((arT.bit_offset-1)>>3) - (bit_offset>>3)+1);
+ remaining_bits_len = arT.remaining_bits_len;
+ bit_offset = arT.bit_offset;
+ }
+ else
+ {
+ /* Has already been processed: ProcessError("csnStreamDissector", Status, pDescr); */
+ return Status;
+ }
}
-
+ pDescr++;
break;
}
@@ -1002,22 +1003,25 @@ csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pDescr, t
proto_item *ti;
proto_tree *test_tree;
- test_tree = proto_tree_add_subtree(tree, tvb, bit_offset>>3, 1, ett_csn1, &ti, pDescr->sz);
-
- csnStreamInit(&arT, bit_offset, remaining_bits_len, ar->pinfo);
- Status = csnStreamDissector(test_tree, &arT, (const CSN_DESCR *)pDescr->descr.ptr, tvb, pvDATA(data, pDescr->offset), ett_csn1);
- if (Status >= 0)
+ if (pDescr->may_be_null && remaining_bits_len == 0)
{
- proto_item_set_len(ti,((arT.bit_offset-1)>>3) - (bit_offset>>3)+1);
- remaining_bits_len = arT.remaining_bits_len;
- bit_offset = arT.bit_offset;
- pDescr++;
- }
- else
- { /* return error code Has already been processed: */
- return Status;
+ proto_tree_add_none_format(tree, hf_null_data, tvb, 0, 0, "[NULL data]: %s Not Present", pDescr->sz);
+ } else {
+ test_tree = proto_tree_add_subtree(tree, tvb, bit_offset>>3, 1, ett_csn1, &ti, pDescr->sz);
+ csnStreamInit(&arT, bit_offset, remaining_bits_len, ar->pinfo);
+ Status = csnStreamDissector(test_tree, &arT, (const CSN_DESCR *)pDescr->descr.ptr, tvb, pvDATA(data, pDescr->offset), ett_csn1);
+ if (Status >= 0)
+ {
+ proto_item_set_len(ti,((arT.bit_offset-1)>>3) - (bit_offset>>3)+1);
+ remaining_bits_len = arT.remaining_bits_len;
+ bit_offset = arT.bit_offset;
+ }
+ else
+ { /* return error code Has already been processed: */
+ return Status;
+ }
}
-
+ pDescr++;
break;
}
diff --git a/epan/dissectors/packet-csn1.h b/epan/dissectors/packet-csn1.h
index 258748df36..1162644885 100644
--- a/epan/dissectors/packet-csn1.h
+++ b/epan/dissectors/packet-csn1.h
@@ -478,6 +478,17 @@ gint16 csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pD
{CSN_TYPE, 0, {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), FALSE, #_MEMBER, NULL, 0, NULL, NULL, 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, NULL, 0, NULL, NULL, NULL}
+
+
+/******************************************************************************
* M_TYPE_LABEL(Par1, Par2, Par3, Par4)
* Same as M_TYPE but allows to define a custom string for the subtree
* <list> ::= {1 <type>} ** 0 ;
diff --git a/epan/dissectors/packet-gsm_rlcmac.c b/epan/dissectors/packet-gsm_rlcmac.c
index c05a267a84..f620a997f7 100644
--- a/epan/dissectors/packet-gsm_rlcmac.c
+++ b/epan/dissectors/packet-gsm_rlcmac.c
@@ -2750,7 +2750,7 @@ CSN_DESCR_BEGIN (Content_t)
/* additions in release 7 */
M_UINT_OR_NULL (Content_t, DTM_Handover_Capability, 1, &hf_content_dtm_handover_capability),
M_NEXT_EXIST_OR_NULL(Content_t, Exist_DownlinkDualCarrierCapability_r7, 1, &hf_content_multislot_capability_reduction_for_dl_dual_carrier_exist),
- 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, &hf_content_flexible_timeslot_assignment),
M_UINT_OR_NULL (Content_t, GAN_PS_HandoverCapability, 1, &hf_content_gan_ps_handover_capability),