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

[Mark Stevans]

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);
     }

(I got the list of modes from searching the assembly language for cases 
where "uvlinesize" was subtracted from the destination).  So far, this 
patch seems to avoid the crashing, though I can't be totally sure 
without days of testing.  At any rate, there's probably a similar fix 
for "mb_x" being zero and horizontal predictions, though that wouldn't 
cause core-dumps.  And there might be similar fixes needed in other codecs.

Is this a sensible diagnosis, or a sensible fix?  Can I do something 
better to avoid these crashes?  Thanks for any comments anyone cares to 
make....