Hi Pascal,
thank you for answer. I saw your commits to follow.c and I hoped for your reply.
450:if( newseq > seq[idx] ) {
I think - Yes. It compares sequence numbers.
459: if ( current->data_len > new_pos ) {
I am sure, that - No. Because it compares length of data from fragment instead
of sequence numbers.
There are some places in check_fragments() and reassemble_tcp() with a "naive"
comparison of sequence numbers:
369: if( sequence < seq[src_index] ) {
I think, they should be replaced with macros from packet-tcp.h 51-55. At least
to be uniformly.
Best Regards,
Pavel Karneliuk
Senior Software Engineer
From: [email protected]
[mailto:[email protected]] On Behalf Of Pascal Quantin
Sent: Friday, March 28, 2014 6:14 PM
To: Developer support list for Wireshark
Subject: Re: [Wireshark-dev] Defect in reassembling TCP stream. Bug and Patch
are available on Bugzilla.
2014-03-28 16:06 GMT+01:00 Pavel Karneliuk
<[email protected]<mailto:[email protected]>>:
Hello,
At first, thank you all for Wireshark. It is amazing tool!
I found a defect and register Bug
9936<https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=9936> - "epan/follow.c
- Incorrect comparing a sequence number of TCP fragment when its value wraps
over uint32_t limit"
A capture file and my patch are attached to bug in Bugzilla.
Patch is a one-line fix:
--- a/epan/follow.c
+++ b/epan/follow.c
@@ -441,7 +441,7 @@ check_fragments( int idx, tcp_stream_chunk *sc, guint32
acknowledged ) {
lowest_seq = current->seq;
}
- if( current->seq < seq[idx] ) {
+ if( LT_SEQ(current->seq, seq[idx]) ) {
guint32 newseq;
/* this sequence number seems dated, but
check the end to make sure it has no more
It is just a replacement a compare operator to wraps-friendly macro. Similar to
code around (with GT_SEQ usage).
What do you think?
Hi Pavel,
while we are at it, shouldn't the comparison done at lines 450 and 459 be
wrapped in a GT_SEQ macro also?
Regards,
Pascal.
___________________________________________________________________________
Sent via: Wireshark-dev mailing list <[email protected]>
Archives: http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
mailto:[email protected]?subject=unsubscribe
