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

[Mark Stevans]

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

Thanks very much for your reply, Michael.  I checked out 
"ff_h264_check_intra_pred_mode", and it doesn't look to me as if it 
works properly.

 From direct examination of "h264_intrapred.asm", there appear to be at 
least three modes that will not function correctly if "mb_y" is zero -- 
they start right out by subtracting "uvlinesize" from "dest_cr" and 
dereferencing that address:

     TOP_DC_PRED8x8 	(5)
     DC_PRED8X8		(0)
     VERT_PRED8X8	(2)

Looking at my trunk copy of "h264.c" from one month ago, if 
"ff_h264_check_intra_pred_mode" is passed mode TOP_DC_PRED8X8 (5) when 
there are no top or left samples available, the body of the second 
conditional (the first being the out-of-range mode test) is executed, 
and changes "mode" to 0 (according to 
http://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Initializing-Arrays). 
  The third conditional body also executes, and changes "mode" to 
TOP_DC_PRED8X8 (5), which, as I said, is unacceptable.

And if you pass LEFT_DC_PRED8X8 (4) to "ff_h264_check_intra_pred_mode" 
with no top samples but with left samples, the second conditional body 
executes and changes "mode" to 0, so the function returns DC_PRED8X8 
(0), which again is unacceptable.

Am I interpreting the actions of this function correctly?  I am not the 
greatest C programmer in the world, so I could be reading the code wrong.

And I now have a stack trace (only took two hours to repro the bug):

ffplay_g!ff_pred8x8_dc_8_mmxext+0x6
ffplay_g!hl_decode_mb_simple_8(struct H264Context * h = 
0x00000000`02803cc0)+0xd12
ffplay_g!ff_h264_hl_decode_mb(struct H264Context * h = 
0x00000000`02803cc0)+0xfe
ffplay_g!decode_slice(struct AVCodecContext * avctx = 
0x00000000`01fede40, void * arg = 0x00000000`03cdfa10)+0x69b
ffplay_g!execute_decode_slices(struct H264Context * h = 
0x00000000`02803cc0, int context_count = 0n1)+0x76
ffplay_g!decode_nal_units(struct H264Context * h = 0x00000000`02803cc0, 
unsigned char * buf = 0x00000000`02e09460 "", int buf_size = 0n2740, int 
parse_extradata = 0n0)+0x1219
ffplay_g!decode_frame(struct AVCodecContext * avctx = 
0x00000000`01fede40, void * data = 0x00000000`023f0a50, int * got_frame 
= 0x00000000`023f0cb8, struct AVPacket * avpkt = 0x00000000`023f09e0)+0x4bb
ffplay_g!frame_worker_thread(void * arg = 0x00000000`023f0940)+0x158
ffplay_g!win32thread_worker(void * arg = 0x00000000`023f0948)+0x39
MSVCR100!endthreadex+0x43
MSVCR100!endthreadex+0xdf
kernel32!BaseThreadInitThunk+0xd
ntdll!RtlUserThreadStart+0x1d

MLS