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

[Graham Bloice <graham.bloice () trihedral com>]

On 18 June 2014 13:12, wsgd <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>:
>
> >  2014-06-16 22:44 GMT+02:00 wsgd <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

___________________________________________________________________________
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