Wireshark · Ethereal-dev: Re: [Ethereal-dev] Crash in ethereal 0.10.8, somewhat reproducible

[Pilz Rene <rene.pilz@xxxxxx>]

Peter Johansson wrote:

> ronnie sahlberg wrote:
>
> > ok   jag avvaktar ett tag.
> >
> > en idee,   det kan eventuellt vara fel i bittorrent dissectorn
> > eftersom det inte verkar vara nagra storre fel pa hur andra
> > dissectorer anvander reassemble.
> >
> > kan du kolla att bittorrent dissectorn ENDAST anropar reassembly OMM
> > packetet verkligen ar fragmenterat.  DVS om hela PDUn redan finns i
> > paketet och darfor ingen reassembly behovs,  da skall inte reassembly
> > rutinerna anropas.
> >
> >
> > reassembly rutinerna ar lite risiga,   de var nar jag skrev
> > reassemble.c  mest ett snabbt hack  men jag fick aldrig tid att skriva
> > om koden battre :-(
> >
> >
> > On Sat, 22 Jan 2005 15:41:22 +0100, Peter Johansson
> > <Peter.Johansson@xxxxxxxxxxxx> wrote:
> >  
> >
> > > ronnie sahlberg wrote:
> > >
> > >   
> > >
> > > > Hej,
> > > >
> > > > Har du en exempel capturefil du kan skicka till mig sa kan jag titta
> > > > pa vad som ar fel.
> > > >
> > > >
> > > >     
> > >
> > > Jag har två capturefiler, vardera om 20MB (jag körde capture till
> > > roterande filer). Om först laddar den ena och sedan den andra kan man
> > > reproducera krashen.
> > > Jag håller på och felsöker och tror jag hittat en workaround för
> > > krashen, tyvärr har jag ännu inte hittat den egentliga orsaken till
> > > problemet.
> > > Den info jag samlat på mig är:
> > > 1. Kraschen är ett faktum (förr eller senare) endast om den trafik
> > > ethereal avkodar innehåller Bittorrent data, det kan alltså vara ett
> > > problem med packet-bittorrent.c (har inte hunnit börja kika på den
> > > filen), se också punkt 4.
> > > 2. Kraschen i reassemble.c som ju sker i ett anrop till memcpy sker
> > > eftersom source (fd_i->data) inte verkar vara allokerat minne,
> > > fd_i->flags anger bla att biten FD_NOT_MALLOCED är satt.
> > > 3. Kraschen inträffar endast när ethereal avkodar insamlat data, dvs
> > > bara om man gör capture med "Update list of packets in realtime" 
> > > påslaget.
> > > 4. Kraschen inträffar endast om Bittorrent avkodaren är med i listan
> > > över "enabled protocols", därför tror jag att orsaken till kraschen
> > > egentligen ligger i packet-bittorent.c.
> > >
> > > Min workaround (som troligtvis ska permanentas även om felet troligtvis
> > > ligger i packet-bittorrent.c) är att skriva om ett fåtal rader i
> > > reassemble.c (se den bifogade bilden), det garanterar att man inte kan
> > > krascha där för att man försöker hantera data som ej är allokerat. Så
> > > vitt jag har förstått bör konsekvensen bli att ethereal kanske inte
> > > avkodar just denna PDU på korrekt sätt. Man bör kanske därför lägga in
> > > någon kod som kan tala om för användaren att så är fallet (kanske en
> > > error popup där framenumret är angivet, etc). Därutöver ska 
> > > naturligtvis
> > > den egentliga orsaken till kraschen grävas fram.
> > >
> > > Vidare tror jag att reassemble.c bör skrivas om såsom den bifogade
> > > bilden visar. Dvs man ska aldrig anropa memcpy om src->flags eller
> > > destination->flags har biten FD_NOT_MALLOCED satt.
> > > Tyvärr rättar ju inte det orsaken till problemet, det ser bara till att
> > > kraschen inte sker. Anledningen till att jag tidigare skrev att jag 
> > > inte
> > > visste vad jag skulle göra med problemet var att jag då inte kunde
> > > reproducera det på ett kontrollerat sätt, det kan jag nu.
> > > Är du fortfarande intresserad av filerna eller vill du avvakta?
> > >
> > > Mvh Peter
> > >   
> I have not forgot about this, I have just been a little bit more busy 
> than usual. I finally tracked down the problem though.
> My conclusion is this:
> Ethereal crashes in reassemble.c because reassemble.c copies data to a 
> memory area that is not yet allocated (fd_i->flags has the 
> FD_NOT_MALLOCED bit set). I have a solution to this (ensuring that a 
> crash does not occur) which I will post once I have done some cleaning 
> up.
>
> The crash in reassemble.c occurs only as the result of a faulty 
> protocol dissector. In this case it is packet-bittorrent.c that is the 
> reason for the crash.
> The Bittorrent dissector registers only a heur-dissector (which should 
> be fine). But once the heur test function detects that this TCP stream 
> is in fact Bittorrent data, it creates a conversation, making sure 
> that all future data in the same TCP stream is decoded by the 
> Bittorrent protocol dissector without the use of the heur test 
> function (this too should be fine I guess).
> The heur test function is capable of telling the calling framework 
> whether the PDU was in fact decoded by this dissector or not by 
> returning TRUE or FALSE
> packet-bittorrent.c. The function that dissects Bittorrent data based 
> on the fact that it belongs to a conversation does not have the 
> opportunity of telling the calling framework that it in fact cannot 
> decode the supplied PDU if necessary. And this is necessary in the 
> rare event that packet-tcp has marked the current PDU with "[TCP 
> previous segment lost]". In this case some data is missing but the 
> Bittorrent dissector still assumes that the first 4 bytes of the PDU 
> denotes the length of the PDU to be dissected. The problem now is that 
> since data was lost, the length is read using a random offset into the 
> original Bittorrent packet (since some data was lost).
> My guess is that this could happen for any dissector that is called 
> since the data belongs to a conversation created by the specific 
> dissector when data has been lost.
>
> Should packet-tcp perhaps not call higher level dissector when the PDU 
> is marked with "[TCP previous segment lost]" or at least not perform 
> the try_conversation_dissector(...) call?
> What would be the better way of ensuring that this does not happen 
> with any of the already existing dissectors?
> Should perhaps the API at hand for dissectors be changed so that when 
> decoding PDU data, the dissector would be able to return TRUE or FALSE 
> in a similar way to the heur functions? This way, any dissector would 
> be able to tell the lower layer dissector that although it should have 
> handled this PDU, it could not.
>
> What is your opinion?
>
> / Peter
>
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@ethereal.
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
Hi

One of the problems which are cause by "Ignore cipher bit" in the old 
LLC-GPRS is a seg-fault in the reassemble.c (The reassembly functions 
are called in the sndcp dissector).  Until now I havn't figured out why, 
but it might be the same issue as peter is working on.

If I know more I will give an update

/rene

--
Dipl-Ing (FH) MSc. C.E René Pilz
ftw. Telekommunications Research Center Vienna http://www.ftw.at
Tech Gate Vienna, Donaucitystraße 1, A-1220 Wien
Mobile: +43 664 8269871 Office: +43 1 5052830-13  Fax: +43 1 5052830-99