Wireshark mailing list archives
Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h
From: Evan Huus <eapache () gmail com>
Date: Thu, 17 Oct 2013 08:27:22 -0400
On Sun, Oct 13, 2013 at 3:54 AM, Jakub Zawadzki <darkjames-ws () darkjames pl> wrote:
On Sun, Oct 13, 2013 at 12:54 AM, <eapache () wireshark org> wrote:http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=52578 User: eapache Date: 2013/10/13 04:54 AM Log:[...]All of this is (theoretically) unnecessary - simply checking the offset without worrying about the length will still catch the very-long-loops, since it is the offset that increases in each iteration, not the length. All that to justify: - implement tvb_ensure_offset_exists which throws an exception if the offset is not within the tvb - use it instead of all the complicated other logic in the pre-short-circuit step of proto_tree_add_item and friends This gives us back about 3/4 of the performance we lost in the original patch. We're still ~2% slower than without any check, but this is the best I can think of right now.On Sun, Oct 13, 2013 at 12:58:12AM -0400, Evan Huus wrote:Jeff (and/or anyone else who cares) I'd appreciate a verification that my logic here is correct, and that I've not accidentally undone your good work.I think it's fine, unless in loops there're some weird offset integer overflows (which returns back to offset + 0), like easy case: proto_tree_add_item(..., offset, 0xfffffffe /* -2 */, ...); offset += (-2); proto_tree_add_item(..., offset, +2, ...); offset += (+2);
You were right, the fuzz-bot found some infinite loops of this type, so I reverted the change. I guess we're just going to have to take the performance hit (sorry Anders). ___________________________________________________________________________ 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:
- Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h Evan Huus (Oct 12)
- Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h Jakub Zawadzki (Oct 13)
- Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h Evan Huus (Oct 13)
- Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h didier (Oct 13)
- Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h Pascal Quantin (Oct 14)
- Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h didier (Oct 18)
- Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h Evan Huus (Oct 13)
- Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h Jakub Zawadzki (Oct 13)
- Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h Jakub Zawadzki (Oct 13)
- Re: [Wireshark-commits] rev 52578: /trunk/epan/ /trunk/epan/: proto.c tvbuff.c tvbuff.h Evan Huus (Oct 17)