Wireshark mailing list archives
Re: Disabling a dissector doesn't seem to quite work.
From: Evan Huus <eapache () gmail com>
Date: Sat, 8 Sep 2012 11:25:15 -0400
On Fri, Sep 7, 2012 at 6:09 PM, Evan Huus <eapache () gmail com> wrote:
On Fri, Sep 7, 2012 at 5:47 PM, Maynard, Chris <Christopher.Maynard () gtech com> wrote:Recently another old proprietary protocol (I’ll call it FOO) was brought to my attention, and I was asked to write a dissector for it. In doing so, I discovered a conflict with another dissector, namely SNA. Initially I thought that simply disabling SNA when analyzing FOO would be good enough for our purposes; however, even after disabling SNA, the FOO dissector was never called. The conflict arises because both dissectors, SNA and FOO, register as follows: dissector_add_uint("llc.dsap", SAP_SNA2, [sna|foo]_handle); I found that even with SNA disabled, I had to comment out the above line of code from the packet-sna.c source file before LLC would successfully hand off dissection to FOO. To illustrate this, I’ve attached a very stripped down and slightly modified version of packet-foo.c, along with a simple foo24.pcap file in case anyone would care to take a look at it. I have since found an alternate solution, but I was surprised that disabling a protocol does not seem to have the completely desired effect. While the packet does not get dissected as SNA, other dissectors are apparently not given the opportunity to dissect the packet even when the SNA dissector is disabled. But beyond LLC and SNA, I was thinking/wondering that maybe this is a general problem affecting all dissectors and that some general solution might be needed?A couple of problems here: - When two protocols register with the same uint value in a table, the second one just overwrites the first. - When two protocols register with the same uint value in a table, we leak memory. It's visible in valgrind, as there are already a couple in the default build. Just run "libtool --mode=execute valgrind --leak-check=full ./tshark" and grep for "dissector_add_uint". - Even disabled protocols are registered, and thus are added to the appropriate table. This can overwrite enabled protocols, depending on registration order. I've got somewhere to be right now, but I'll try and do further analysis this weekend.
There is already a (commented-out) function called dissector_add_uint_sanity_check which does warn on duplicate port registrations and on registrations to port 0. It produces 157 warnings when enabled in the default build. I don't know how many duplicate string registrations there are, but there are definitely some based on valgrind's output. I have fixed the memory leaks (about 2K by default) in revision 44814. This doesn't stop us from overwriting existing registrations, it just prevents us from leaking memory when we do overwrite existing registrations. The fundamental problem is that registration happens once right when the program starts, while protocols can be enabled/disabled arbitrarily while an instance is running. Triggering a reregistration on a protocol enable/disable would be painful, and I don't even know if it would solve the problem. A possible solution would be to store a linked list of dtbl_entries for each registration instead of the single entry we currently store. Then dissector_try_uint() can iterate through and try all of the dtbl_entries whose associated protocols are marked as enabled. This might lead to odd behaviour when multiple protocols are enabled for the same port though... we have no way of knowing which one to pick, so we'd have to be careful to avoid bad things (multiple protocols dissecting the same tvb subset, or worse?). New-style dissectors that do heuristics first might make that manageable though. Thoughts? Evan ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Current thread:
- Disabling a dissector doesn't seem to quite work. Maynard, Chris (Sep 07)
- Re: Disabling a dissector doesn't seem to quite work. Evan Huus (Sep 07)
- Re: Disabling a dissector doesn't seem to quite work. Evan Huus (Sep 08)
- Re: [Wireshark-dev] Disabling a dissector doesn't seem to quite work. Christopher Maynard (Sep 15)
- Re: Disabling a dissector doesn't seem to quite work. Guy Harris (Sep 15)
- Re: Disabling a dissector doesn't seem to quite work. Joerg Mayer (Sep 16)
- Re: Disabling a dissector doesn't seem to quite work. Guy Harris (Sep 16)
- Re: Disabling a dissector doesn't seem to quite work. Joerg Mayer (Sep 17)
- Re: Disabling a dissector doesn't seem to quite work. Evan Huus (Sep 08)
- Re: Disabling a dissector doesn't seem to quite work. Evan Huus (Sep 07)
- Re: [Wireshark-dev] Disabling a dissector doesn't seem to quite work. Christopher Maynard (Sep 15)
- Re: Disabling a dissector doesn't seem to quite work. Evan Huus (Sep 15)
- Re: Disabling a dissector doesn't seem to quite work. Bill Meier (Sep 15)
- Re: Disabling a dissector doesn't seem to quite work. Jeff Morriss (Sep 17)