[FFmpeg-devel] Possible crasher bug when decoding unreliable H264 data

[Mark Stevans]

On 6/21/2013 11:55 AM, Michael Niedermayer wrote:
> On Fri, Jun 21, 2013 at 06:21:59AM -0700, Mark Stevans wrote:
>> On 6/21/2013 3:24 AM, Michael Niedermayer wrote:
>>> On Fri, Jun 21, 2013 at 02:21:24AM -0700, Mark Stevans wrote:
>>>> Sorry I don't have a complete stack trace on this one, but I didn't
>>>> save it from the last crash, and it takes about 4-6 hours to get the
>>>> bug to occur again.  And I don't have sample data, because I'm
>>>> playing a 100 KB/s stream
>>>> (rtmp://planeta-online.tv:1936/live/channel_22) with FFPlay for
>>>> hours to get the bug to happen.
>>>>
>>>> When playing unreliable H264 streams with FFPlay, I seem to get
>>>> core-dumps randomly every few hours.  The exact location is usually
>>>> the second instruction of "pred8x8_top_dc_8_mmxext" in
>>>> "h264_intrapred.asm", where it dereferences "dest_cr" after
>>>> subtracting "uvlinesize" from it, as called from the line reading
>>>>
>>>>      h->hpc.pred8x8[h->chroma_pred_mode](dest_cr, uvlinesize);
>>>>
>>>> in "h264_mb_template.c".  "uvlinesize" is typically something like
>>>> 320 at the time of crash, with "mb_y" zero.
>>>>
>>>> My take on this is that, when presented with garbaged stream data,
>>>> the H264 frame decoder sometimes tries to perform predictions that
>>>> involve higher rows (lower memory addresses): if "mb_y" happens to
>>>> be zero (the top row), this means that it tries to read memory from
>>>> "negative rows", addresses a few hundred bytes before the beginning
>>>> of the legitimate frame data.  Often, those addresses point to
>>>> harmless random bytes, but occasionally it actually points to
>>>> unmapped memory pages, causing Access Violations.
>>>>
>>>> My fix is crude: changing the code to read
>>>>
>>>>      if (mb_y <= 0 && (
>>>>          h->chroma_pred_mode == TOP_DC_PRED8x8 ||
>>>>          h->chroma_pred_mode == DC_PRED8x8 ||
>>>>          h->chroma_pred_mode == VERT_PRED8x8)
>>>>      ) {
>>>>          av_log(NULL, AV_LOG_WARNING, "Skipping prediction involving
>>>> previous data rows because mb_y is zero\n");
>>>>      } else {
>>>>          h->hpc.pred8x8[h->chroma_pred_mode](dest_cb, uvlinesize);
>>>>          h->hpc.pred8x8[h->chroma_pred_mode](dest_cr, uvlinesize);
>>>>      }
>>>
>>> Please see ff_h264_check_intra_pred_mode(), It already checks and
>>> sets a valid mode
>>> Thus the issue most likely is elsewhere
>>
>> Michael: assuming I understand the current code (a big assumption),
>
> function fixed

Thanks very much for your help on this one, Michael.  I greatly prefer 
your solid patch to my frenzied guesswork delta :-)

MLS