Re: 1.12.0rc2 : tshark crash when message is on 3 packets

[wsgd <wsgd () free fr>]

Le 19/06/2014 00:14, Pascal Quantin a écrit :
> Indeed the sample code in README.dissector was not updated to make use 
> of the new style dissectors (while packet-PROTOABBREV.c sample code is 
> updated).
> As Graham stated, the new style (indicating the number of bytes 
> consumed by the dissector) should be used. The old one is still here 
> because not all dissectors were converted to new style (quite a long 
> and painful task... :) ).
>
> Pascal.
>
>
> 2014-06-18 23:59 GMT+02:00 Graham Bloice <graham.bloice () trihedral com 
> <mailto:graham.bloice () trihedral com>>:
>
>     On 18 June 2014 13:12, wsgd <wsgd () free fr <mailto:wsgd () free fr>>
>     wrote:
>
>         Le 18/06/2014 00:41, Pascal Quantin a écrit :
> >         2014-06-18 0:11 GMT+02:00 Pascal Quantin
> >         <pascal.quantin () gmail com <mailto:pascal.quantin () gmail com>>:
> >
> >             2014-06-16 22:44 GMT+02:00 wsgd <wsgd () free fr
> >             <mailto:wsgd () free fr>>:
> >
> >                 Hello,
> >
> >                 My protocol (only to test this problem) specifications :
> >                 tcp port 20640
> >                 message is 5 bytes long
> >
> >
> >
> >                 command line : tshark -r pb.cap  -T text -V
> >                 --> crash (see pb.1.12.0.txt)
> >                 **
> >                 ERROR:print.c:838:get_field_data: code should not be
> >                 reached
> >
> >                 This application has requested the Runtime to
> >                 terminate it in an unusual way.
> >                 Please contact the application's support team for
> >                 more information.
> >
> >
> >
> >                 wireshark does not crash and display is ok
> >                 tshark 1.10.6 does not crash and display is ok (see
> >                 pb.1.10.6.txt)
> >
> >
> >                 Plugin dissector code is into packet-tcp-5-bytes.c
> >
> >
> >
> >                 Regards,
> >                 Olivier
> >
> >
> >             Hi Olivier,
> >
> >             thanks for the report.
> >             This is a regression introduced by g21e0a63b2 commit for
> >             bug 9169. I proposed a fix (not calling the data
> >             dissector when a subdissector claims that the current TCP
> >             fragment needs more desegmentation) here:
> >             https://code.wireshark.org/review/2350
> >
> >             Regards,
> >             Pascal.
> >
> >
> >         Hi Olivier,
> >
> >         as Evan noted in the review of my patch, the data dissector
> >         should not even be called as your dissector accepted the
> >         packet. It appears that there is a small bug in your current
> >         code. In function dissect_tcp_5_bytes(), replacing the line 30:
> >                     return  offset;
> >         by
> >                     return  offset + available;
> >         does not trigger the crash.
> >
> >         With the previous code, your dissector was returning the
> >         value 0 for frame 4, like if the packet was rejected. But at
> >         the same time you were considering the packet as acceptable
> >         and changing the pinfo->desegment_len, leading to an
> >         inconsistent state that should have been caught by a missing
> >         check in packet-tcp.c
> >
> >         Regards,
> >         Pascal.
>         Hi Pascal,
>
>
>         Ok, my fault.
>         Sorry for the inconvenience.
>
>
>
>         Question : the dissect function must return void or int ?
>         I know both versions exist.
>         Is there one deprecated or one better ?
>
>
>
>         Only void dissect function into
>         http://www.wireshark.org/docs/wsdg_html_chunked/ChDissectAdd.html.
>
>         void dissect function into
>         https://code.wireshark.org/review/gitweb?p=wireshark.git;a=blob_plain;f=doc/README.dissector
>         :
>
>         static void dissect_cstr(tvbuff_t * tvb, packet_info * pinfo, proto_tree * tree)
>
>     I can't see it in the docs anywhere (which is an omission that
>     should be corrected), but epan\packet.h holds a little information
>     about the types of dissector functions.  New code should be using
>     the new dissector type.
>
>     Graham

Thank you both.
Olivier

___________________________________________________________________________
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