Wireshark mailing list archives
Re: [Wireshark-dev] Disabling a dissector doesn't seem to quite work.
From: Christopher Maynard <Christopher.Maynard () gtech com>
Date: Sat, 15 Sep 2012 17:33:26 +0000 (UTC)
Evan Huus <eapache@...> writes:
A couple of problems here: - When two protocols register with the same uint value in a table, the second one just overwrites the first.
If the first one is disabled, I'd say that's OK, but if not, then maybe the better thing to do is not to register the new one? At least this way there would be a way to disable dissectors, thus giving other dissectors a chance at decoding packets, even if this means having to restart Wireshark to achieve this. But I'd say this would probably fall into a decent enough temporary solution, but it's not ideal.
- Even disabled protocols are registered, and thus are added to the appropriate table. This can overwrite enabled protocols, depending on registration order.
Yeah, it would be nice if at least this situation could be avoided.
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.
My understanding of dissectors registering to port 0 was simply a method for allowing "Decode As" to work. Those *probably* aren't a problem. The others would likely not yield expected results though.
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.
Thanks! :)
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.
I agree, this would probably be tedious, essentially adding de-registration routines to all dissectors, which are then called whenever the respective protocol is disabled. But I would think that it would solve the basic problem though, even if it means having to potentially disable multiple protocols in order to finally get to the one you want, depending upon how many dissectors are registered to the same uint value. That isn't all that elegant of course, but it would work, which is better than what we have today.
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
Maintaining a linked list seems like a good idea to me and a lot easier than adding de-registration routines to 1000+ dissectors. Until all dissectors are fully converted to new-style ones, maybe they could all be "forcibly" converted by simply having them return the number of bytes in the tvb that are handed to them? Then slowly they could really be converted to actually perform heuristics and return the actual number of bytes dissected. - Chris ___________________________________________________________________________ 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)