From patchwork Sun May 1 21:39:28 2005 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Reinhard Nissl X-Patchwork-Id: 11863 Received: from imap.gmx.net ([213.165.64.20] helo=mail.gmx.net) by www.linuxtv.org with smtp (Exim 4.34) id 1DSMAl-0007wq-WF for vdr@linuxtv.org; Sun, 01 May 2005 23:40:20 +0200 Received: (qmail invoked by alias); 01 May 2005 21:39:38 -0000 Received: from dialin-145-254-246-200.arcor-ip.net (EHLO [192.168.101.15]) [145.254.246.200] by mail.gmx.net (mp005) with SMTP; 01 May 2005 23:39:38 +0200 X-Authenticated: #527675 Message-ID: <42754C90.7050501@gmx.de> Date: Sun, 01 May 2005 23:39:28 +0200 From: Reinhard Nissl User-Agent: Mozilla Thunderbird 1.0 (X11/20041206) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Klaus Schmidinger's VDR X-Y-GMX-Trusted: 0 Subject: [vdr] vdr-1.3.23: BUG: cPesAssembler dropped data resulting in delayed crash of VDR X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Klaus Schmidinger's VDR List-Id: Klaus Schmidinger's VDR List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 01 May 2005 21:40:20 -0000 Status: O X-Status: X-Keywords: X-UID: 1880 Hi, while recording the radio channel "Bayern 1" on Astra 19.2E from 0900 to 1000 this morning, I've discoverd that VDR crashed at about 0932. After debugging the whole afternoon I've recently found the bug that caused this crash. It's cPesAssembler that drops some data in certain circumstances. The crash has nothing todo with recording this channel, but with the transfer mode I'm using with vdr-xine, as it makes use of cPesAssembler. BTW: To reproduce this crash just tune to the mentioned channel in transfer mode and wait for about 33 minutes. The bug has to do with cPesAssembler's length member, as it conforms to this equation: length == 0 || length >=4 It's therefore impossible for cPesAssembler to store any fragments of less than 4 bytes. But the problem is, that these bytes have modified it's shift register "tag" which leads to wrong synchronisation on the next fragment. For this channel, it seems to happen that cPesAssembler sees data blocks that end with the sequence 00 00 01. These 3 bytes are stored in cPesAssembler's "tag" member. But when the next data block arrives, these bytes are ignored, as "length" is still 0, and further data is dropped, as it needs to sychronize on the next start code (00 00 01). When the next fragment is to be stored, it results in "data" beeing something like that 00 00 01 00 00 01 c0 ... As a result, the expected length evaluates to 6 bytes, but "length" containes already more than that (in the above case it was 79). This finally leads to "Rest" beeing -73 which then causes a crash in memcpy. The attached patch fixes this issue by introducing a new variable "hasFragment". It helps to recognize that 1 to 3 bytes are already stored in cPesAssembler. Feel free to drop this new variable and use "length" instead. Where length is checked for beeing 0, it should be checked for beeing < 4 and where "hasFragment" is set to true, "length" should be set to any value != 0 and < 4. Bye. --- ../vdr-1.3.23-orig/device.c 2005-02-27 14:55:15.000000000 +0100 +++ device.c 2005-05-01 22:59:18.869069399 +0200 @@ -27,6 +27,7 @@ private: uint32_t tag; int length; int size; + bool hasFragment; bool Realloc(int Size); public: cPesAssembler(void); @@ -39,6 +40,7 @@ public: void Put(uchar c); void Put(const uchar *Data, int Length); bool IsPes(void); + bool HasFragment(void); }; cPesAssembler::cPesAssembler(void) @@ -57,6 +59,12 @@ void cPesAssembler::Reset(void) { tag = 0xFFFFFFFF; length = 0; + hasFragment = false; +} + +bool cPesAssembler::HasFragment(void) +{ + return hasFragment || length > 0; } bool cPesAssembler::Realloc(int Size) @@ -67,6 +75,7 @@ bool cPesAssembler::Realloc(int Size) if (!data) { esyslog("ERROR: can't allocate memory for PES assembler"); length = 0; + hasFragment = false; size = 0; return false; } @@ -77,6 +86,7 @@ bool cPesAssembler::Realloc(int Size) void cPesAssembler::Put(uchar c) { if (!length) { + hasFragment = true; tag = (tag << 8) | c; if ((tag & 0xFFFFFF00) == 0x00000100) { if (Realloc(4)) { @@ -985,7 +997,7 @@ int cDevice::PlayPes(const uchar *Data, return 0; } int Result = 0; - if (pesAssembler->Length()) { + if (pesAssembler->HasFragment()) { // Make sure we have a complete PES header: while (pesAssembler->Length() < 6 && Length > 0) { pesAssembler->Put(*Data++);