Re: Question on payload reassembly

[John Dunlop <jdunlop () maxlinear com>]

Thanks Roland/Jeff for the responses, much appreciated.

So spent a bit of time debugging this and it looks like we fail sometimes to return a valid frag_msg from a call to 
fragment_add_seq_check() when more_frags is set to false.  In the case this happens I am currently failing to see much 
difference in how the id and frag_number are controlled i.e. they are unique and in-sequence respectively.  The failing 
point is that we receive NULL from lookup_fd_head() from within fragment_add_seq_common() so I assume 
g_hash_table_lookup_extended() fails in someway.

So perhaps I need to follow your advice Roland and use fragment_add_seq_offset() at the beginning of each fragment 
sequence, though I am unclear why this should be needed.  Or I need to follow the packet-mp2t.c code which manipulates 
pinfo->src and pinfo->dst?

Otherwise I have added some code in my payload parse function to make use of fragment_get() to infer when the packets 
payload has already being parsed and hence don’t need to re-check the legitimacy of the begin/end/seq-numbers.  I am 
not at the point of testing this since we are failing on the initial file parse now.

Regards,

John

From: wireshark-dev-bounces () wireshark org [mailto:wireshark-dev-bounces () wireshark org] On Behalf Of Roland Knall
Sent: 28 July 2016 15:39
To: Developer support list for Wireshark
Subject: Re: [Wireshark-dev] Question on payload reassembly

Hi

Just a short question, does your sequence counter repeat? If so, this can be an issue. Also, for the openSAFETY 
dissector it only worked properly, after I implemented fragment_add_seq_offset, so it will allways count internally 
beginning with 0. You can see that in line 1272 of packet-opensafety.c

regards,
Roland

On Thu, Jul 28, 2016 at 3:38 PM, Jeff Morriss <jeff.morriss.ws () gmail com<mailto:jeff.morriss.ws () gmail com>> wrote:


On Thu, Jul 28, 2016 at 8:35 AM, John Dunlop <jdunlop () maxlinear com<mailto:jdunlop () maxlinear com>> wrote:

Hi,



Hope someone can help me with a question of payload reassembly.



First up, I have been trawling the e-mail archives to find an equivalent answer and was wondering if there is a better 
way of searching the e-mail archives than opening up each individual month/year?

Personally I use Google with a search string like:
    what I'm interested in site://wireshark.org<http://wireshark.org>


Now my actual question is that I am dissecting  a packet payload which is split up into fragments with specific chunks 
as:

Begin

Middle (no begin/end flagged, so can be multiple)

End



I have a simple state machine that checks these transitions and keeps fragment counts so I can then call 
fragment_add_seq_check() with an appropriate unique id and an incrementing (from zero) frag_number.  I know the size of 
the individual fragments and there is a sequence number that increments on each packet, though a packet can have 
multiple fragments for the same or different channels .



This appears to ‘initially’ work ok from the various log prints I had added to check returns from 
process_reassembled_data() and the actual reassembled TVB size.



The problem I have, and this is probably my fundamental misunderstanding, is that it works on the initial pass through 
the packets but breaks horribly when I click on an individual packet as we are mid fragments. I also notice that 
wireshark parses the whole file once and then parses again the visible packets in the summary window, this also fails 
as the 1st packet is parsed again after the last which could be in any state of fragmentation.



I suppose I am thinking if we have parsed the payload once for a given packet/fragment we should not parse and 
reassemble again but somehow look-up what reassembled payload it belongs to? Using something like fragment_get() ?

Hmm, the reassembly routines should take care of this for you.  See the first 'if' statement in 
`fragment_add_seq_check_work()` (in epan/reassemble.c): it checks if the current frame has already been dissected and, 
if so, it skips reassembly and just returns what was stored from the first pass.

It sounds like you are but are you *really* sure you're doing all the reassembly on the first pass (e.g., the 
reassembly calls aren't buried under an `if(tree)` for example)?
I suppose this won't answer your question but hopefully it might give you a direction to look in...

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org<mailto:wireshark-dev () wireshark org>>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org<mailto:wireshark-dev-request () wireshark 
org>?subject=unsubscribe

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe