Wireshark · Wireshark-dev: [Wireshark-dev] Segmentation fault with SVN revision 36377 when configuring a specific DLT_USER

[Pascal Quantin <pascal.quantin@xxxxxxxxx>]

Hi,

starting from revision 36377, I get a systematic segmentation fault when calling Wireshark with the following command line:
tshark -o "uat:user_dlts:\"User 15 (DLT=162)\",\"lte-rrc.ul.dcch\",\"0\",\"\",\"0\",\"\"" -r "./temp.pcap" -V > "./output_pcap.txt"

The backtrace is:
#0  0xb619b0fd in proto_get_protocol_name (proto_id=-1) at proto.c:4485
#1  0xb68a4362 in dissect_user (tvb=0xa4e9f68, pinfo=0xbffb2a50, tree=0x0) at packet-user_encap.c:127
#2  0xb618ed65 in call_dissector_through_handle (handle=0x9e8d0b8, tvb=0xa4e9f68, pinfo=0xbffb2a50, tree=0x0) at packet.c:395

#3  0xb618f53e in call_dissector_work (handle=0x9e8d0b8, tvb=0xa4e9f68, pinfo_arg=0xbffb2a50, tree=0x0, add_proto_name=1) at packet.c:486
#4  0xb6190749 in dissector_try_uint_new (sub_dissectors=0x9aa1ef0, uint_val=60, tvb=0xa4e9f68, pinfo=0xbffb2a50, tree=0x0, add_proto_name=1) at packet.c:898

#5  0xb61907b1 in dissector_try_uint (sub_dissectors=0x9aa1ef0, uint_val=60, tvb=0xa4e9f68, pinfo=0xbffb2a50, tree=0x0) at packet.c:924
#6  0xb6463d18 in dissect_frame (tvb=0xa4e9f68, pinfo=0xbffb2a50, parent_tree=0x0) at packet-frame.c:348

#7  0xb618ed65 in call_dissector_through_handle (handle=0x9aa1f60, tvb=0xa4e9f68, pinfo=0xbffb2a50, tree=0x0) at packet.c:395
#8  0xb618f53e in call_dissector_work (handle=0x9aa1f60, tvb=0xa4e9f68, pinfo_arg=0xbffb2a50, tree=0x0, add_proto_name=1) at packet.c:486

#9  0xb618f6da in call_dissector (handle=0x9aa1f60, tvb=0xa4e9f68, pinfo=0xbffb2a50, tree=0x0) at packet.c:1828
#10 0xb6191235 in dissect_packet (edt=0xbffb2a48, pseudo_header=0x821942c, pd=0x82194bc "$", fd=0xa0fd178, cinfo=0x0) at packet.c:326

#11 0xb6186a3e in epan_dissect_run (edt=0xbffb2a48, pseudo_header=0x821942c, data="" "$", fd=0xa0fd178, cinfo=0x0) at epan.c:201
#12 0x0807d033 in add_packet_to_packet_list (fdata=0xa0fd178, cf=0x82193a0, dfcode=0x0, filtering_tap_listeners=0, tap_flags=0, pseudo_header=0x821942c, buf=0x82194bc "$", refilter=1, 

    add_to_packet_list=1) at file.c:1119
#13 0x0807d42b in rescan_packets (cf=0x82193a0, action="" "Reprocessing", action_item=0x81ae882 "all packets", refilter=1, redissect=1) at file.c:1804

#14 0x0807d8f7 in cf_redissect_packets (cf=0x82193a0) at file.c:1558
#15 0x08092132 in redissect_packets () at main.c:3757

After digging a little bit, it is due to the way LTE RRC dissectors are registered:

#define PNAME  "LTE Radio Resource Control (RRC) protocol"
#define PSNAME "LTE RRC"
#define PFNAME "lte_rrc"
 /* Register protocol */
  proto_lte_rrc = proto_register_protocol(PNAME, PSNAME, PFNAME);

  /* These entry points will first create an lte_rrc root node */
  register_dissector("lte_rrc.dl_ccch", dissect_lte_rrc_DL_CCCH, proto_lte_rrc);
  register_dissector("lte_rrc.dl_dcch", dissect_lte_rrc_DL_DCCH, proto_lte_rrc);

  register_dissector("lte_rrc.ul_ccch", dissect_lte_rrc_UL_CCCH, proto_lte_rrc);
  register_dissector("lte_rrc.ul_dcch", dissect_lte_rrc_UL_DCCH, proto_lte_rrc);
  /* Register fields and subtrees */

  proto_register_field_array(proto_lte_rrc, hf, array_length(hf));
  proto_register_subtree_array(ett, array_length(ett));
  /* Register the dissectors defined in lte-rrc.conf */
/*--- Included file: packet-lte-rrc-dis-reg.c ---*/

#line 1 "packet-lte-rrc-dis-reg.c"
  new_register_dissector("lte-rrc.bcch.bch", dissect_BCCH_BCH_Message_PDU, proto_lte_rrc);
  new_register_dissector("lte-rrc.bcch.dl.sch", dissect_BCCH_DL_SCH_Message_PDU, proto_lte_rrc);

  new_register_dissector("lte-rrc.mcch", dissect_MCCH_Message_PDU, proto_lte_rrc);
  new_register_dissector("lte-rrc.pcch", dissect_PCCH_Message_PDU, proto_lte_rrc);
  new_register_dissector("lte-rrc.dl.ccch", dissect_DL_CCCH_Message_PDU, proto_lte_rrc);

  new_register_dissector("lte-rrc.dl.dcch", dissect_DL_DCCH_Message_PDU, proto_lte_rrc);
  new_register_dissector("lte-rrc.ul.ccch", dissect_UL_CCCH_Message_PDU, proto_lte_rrc);
  new_register_dissector("lte-rrc.ul.dcch", dissect_UL_DCCH_Message_PDU, proto_lte_rrc);

  new_register_dissector("lte-rrc.ue_cap_info", dissect_UECapabilityInformation_PDU, proto_lte_rrc);
  new_register_dissector("lte-rrc.ue_eutra_cap", dissect_lte_rrc_UE_EUTRA_Capability_PDU, proto_lte_rrc);

Here the call to proto_get_id_by_filter_name() return -1 as "lte-rrc.ul.dcch" does not match the "lte_rrc" string registered.

As this is not the only dissector like this (at least RRC dissector is done the same way), I would be tempted to protect WS against the crash with something like this:

Index: epan/dissectors/packet-user_encap.c
===================================================================
--- epan/dissectors/packet-user_encap.c    (revision 36377)
+++ epan/dissectors/packet-user_encap.c    (working copy)

@@ -116,16 +116,22 @@
         call_dissector(encap->header_proto, hdr_tvb, pinfo, tree);
         offset = encap->header_size;
         if (encap->header_proto_name) {
-            const char *proto_name = proto_get_protocol_name(proto_get_id_by_filter_name(encap->header_proto_name));

-            proto_item_append_text(item, ", Header: %s (%s)", encap->header_proto_name, proto_name);
+            int id = proto_get_id_by_filter_name(encap->header_proto_name);
+            if (id != -1) {

+                const char *proto_name = proto_get_protocol_name(id);
+                proto_item_append_text(item, ", Header: %s (%s)", encap->header_proto_name, proto_name);
+            }
         }

     }
     
     payload_tvb = tvb_new_subset(tvb, encap->header_size, len, len);
     call_dissector(encap->payload_proto, payload_tvb, pinfo, tree);
     if (encap->payload_proto_name) {
-        const char *proto_name = proto_get_protocol_name(proto_get_id_by_filter_name(encap->payload_proto_name));

-        proto_item_append_text(item, ", Payload: %s (%s)", encap->payload_proto_name, proto_name);
+        int id = proto_get_id_by_filter_name(encap->payload_proto_name);
+        if (id != -1) {

+            const char *proto_name = proto_get_protocol_name(id);
+            proto_item_append_text(item, ", Payload: %s (%s)", encap->payload_proto_name, proto_name);
+        }
     }
 
     if (encap->trailer_size) {

@@ -133,8 +139,11 @@
         call_dissector(encap->trailer_proto, trailer_tvb, pinfo, tree);
         offset = encap->trailer_size;
         if (encap->trailer_proto_name) {
-            const char *proto_name = proto_get_protocol_name(proto_get_id_by_filter_name(encap->trailer_proto_name));

-            proto_item_append_text(item, ", Trailer: %s (%s)", encap->trailer_proto_name, proto_name);
+            int id = proto_get_id_by_filter_name(encap->trailer_proto_name);
+            if (id != -1) {

+                const char *proto_name = proto_get_protocol_name(id);
+                proto_item_append_text(item, ", Trailer: %s (%s)", encap->trailer_proto_name, proto_name);
+            }
         }

     }
 }


What do you guys think ?

Regards,
Pascal.