[Pascal Quantin <pascal.quantin@xxxxxxxxx>]

Hi Graham,

Le mar. 27 mars 2018 à 18:40, Graham Bloice <graham.bloice@xxxxxxxxxxxxx> a écrit :

On 27 March 2018 at 16:42, <david_aggeler@xxxxxxxxxx> wrote:

 

I stepped through a DICOM capture multiple times, and the result is a little surprising.

I’ve attached an analysis document to the bug and the minimal .pcapng to reproduce.

 

To me, it looks like the re-assembly does not work anymore, when the TCP traffic has missing frames and retransmits.

 

At DICOM dissector level I do a plain ‘get more’

 

pinfo->desegment_len = xxxxxx;

return tvb_captured_length(tvb);

 

The re-entry happens far too early (and not with the requested amount of bytes). So it’s like its loosing track.

 

Therefore I looked closer at versions affected. And 1.12.13 is o.k., while, 2.0.0 is not.

To me the wrong decision happens at line Line 3077 (v2.5) in packet-tcp.c

Basically, I’m missing one frame, but it still considers seq to be big enough.

 

desegment_tcp()

..

if (msp && msp->seq <= seq && msp->nxtpdu > seq) {

..

 

 

But this area has not changed between 1.12 and 2.0, so its higher up. I did not managed to create a debug environment for 1.12 yet to narrow in, and don’t understand packet-tcp well enough.

 

Now my question

 

• How good is 2.x supposed to be in TCP level reassembly in case of missing packets and retransmits?
• Can anybody help?

 

Regards

David

IMHO reassembly breaks once you have missing segments.  Retransmits are flagged as errors and not normally handed off to subdissectors, and if you have a partial retransmit, i.e. some of the segment is a retransmission, the rest is new, then the new stuff also doesn't get handed off.

I have a local work in progress to detect partial retransmissions (done) and then ensure subdissectors are handed the new data (not done).  Unfortunately the code is quite complex and finding sufficient spare time to align my mind to the task and code complexity and then fix it is proving difficult.

I remember that at some point we changed the TCP preference allowing to call subdissectors for error packets from true to false. Was it for 2.0? Based on your email, I'm thinking that maybe David could toggle this preference and see whether he gets the same dissection as 1.12 or not.

BR,

Pascal.