[John Dunlop <jdunlop@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] 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@xxxxxxxxx> wrote:

 

 

On Thu, Jul 28, 2016 at 8:35 AM, John Dunlop <jdunlop@xxxxxxxxxxxxx> 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

 

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 1^st 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@xxxxxxxxxxxxx>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe