[vdr] Re: Nasty bug in StillPicture()

[Oliver Endriss <o.endriss@gmx.de>]

On Tuesday 14 October 2003 23:15, Stefan Huelswitt wrote:
> Hi,
> while setting marks on my recordings I noticed that from time to
> time VDR segfaults while moving the marks.
> 
> I tracked this down to bad parsing in
> cDvbDevice::StillPicture(). If the Data field contains non-0xEx
> packets these packets are scanned byte by byte. If the data of
> e.g. a 0xC0 audio packet contains the sequence 00 00 01 ea, this
> is misdetected as a video packet. In most cases this leads to an
> overflow of the allocated buff.

Oops, I wasn't aware that audio packets may contain start codes. :-(

> Solution: skip detected PES packets in one jump.
> 
> Find the attached patch (without whitespace changes. Sorry
> linenumbers may be off too).

But this patch is not quite correct either. The length field ('len') is
present for PES headers only, i.e. 00 00 01 xx (xx = BD...EF).
For robustness I slightly modified your patch and reformatted it
according to Klaus' coding style:

--- dvbdevice.c.org	Sat Sep  6 15:19:33 2003
+++ dvbdevice.c	Wed Oct 15 01:24:54 2003
@@ -915,21 +915,27 @@ void cDvbDevice::StillPicture(const ucha
         return;
      int i = 0;
      int blen = 0;
-     while (i < Length - 4) {
-           if (Data[i] == 0x00 && Data[i + 1] == 0x00 && Data[i + 2] == 0x01 && (Data[i + 3] & 0xF0) == 0xE0) {
-              // skip PES header
-              int offs = i + 6;
+     while (i < Length - 6) {
+           if (Data[i] == 0x00 && Data[i + 1] == 0x00 && Data[i + 2] == 0x01) {
               int len = Data[i + 4] * 256 + Data[i + 5];
-              // skip header extension
-              if ((Data[i + 6] & 0xC0) == 0x80) {
-                 offs += 3;
-                 offs += Data[i + 8];
-                 len -= 3;
-                 len -= Data[i + 8];
+              if ((Data[i + 3] & 0xF0) == 0xE0) { // video packet
+                 // skip PES header
+                 int offs = i + 6;
+                 // skip header extension
+                 if ((Data[i + 6] & 0xC0) == 0x80) {
+                    offs += 3;
+                    offs += Data[i + 8];
+                    len -= 3;
+                    len -= Data[i + 8];
+                    }
+                 memcpy(&buf[blen], &Data[offs], len);
+                 i = offs + len;
+                 blen += len;
                  }
-              memcpy(&buf[blen], &Data[offs], len);
-              i = offs + len;
-              blen += len;
+              else if (Data[i + 3] >= 0xBD && Data[i + 3] <= 0xDF) // other PES packets
+                 i += len+6;
+              else
+                 i++;
               }
            else
               i++;


Oliver


-- 
Info:
To unsubscribe send a mail to ecartis@linuxtv.org with "unsubscribe vdr" as subject.