[PATCH] Re: [Ffmpeg-devel] FFmpeg: H.264 decoding issue

[Michael Niedermayer]

Hi

On Tue, Feb 20, 2007 at 12:39:34AM +0100, Matthias Hopf wrote:
> On Feb 09, 07 00:25:12 +0100, Reinhard Nissl wrote:
> > I've uploaded a H.264 sample file which isn't decoded properly. The file
> > seems to be OK as it can be properly decoded on Windows using
> > Cyberlink's HD264Pack.
> > 
> > 	mplayer -demuxer h264es -vc ffh264 /video/luxe-hd.es.h264
> > 
> > [h264 @ 0x84cd0a0]concealing 2720 DC, 2720 AC, 2720 MV errors
> > Xlib: unexpected async reply (sequence 0x70)!
> > Segmentation fault
> 
> Find two patches attached that should fix *some* of h264 PAFF issues.
> Note that they don't fix PAFF at all, but add safety checks.
> 
> The issues fixed by these patches are IMHO not security relevant. While
> the first is a buffer overflow, it only overwrites frames, and only with
> data from frame edges, which are not user specificable AFAIK. The second
> is a plain old fashioned NULL pointer access, so appart from DoS no
> security issue.
> 
> Note also that the file I tested this with plays fine now on my
> Pentium M, but still crashes on my Athlon XP. Though I guess this is
> some side effect and not an architecture specific problem. So these are
> certainly only the first of a series of patches.
> 
> Please test with your file
> 
> Matthias
> 
> -- 
> Matthias Hopf <mhopf at suse.de>       __        __   __
> Maxfeldstr. 5 / 90409 Nuernberg    (_   | |  (_   |__         mat at mshopf.de
> Phone +49-911-74053-715            __)  |_|  __)  |__  labs   www.mshopf.de

> Index: libavcodec/utils.c
> ===================================================================
> --- libavcodec/utils.c	(revision 8020)
> +++ libavcodec/utils.c	(working copy)
> @@ -278,7 +278,7 @@
>  
>          if(!(s->flags&CODEC_FLAG_EMU_EDGE)){
>              w+= EDGE_WIDTH*2;
> -            h+= EDGE_WIDTH*2;
> +            h+= EDGE_WIDTH*2+1;		// +1 for potential interlace (MPV_frame_start)

why?


[...]
> Index: libavcodec/mpegvideo.c
> ===================================================================
> --- libavcodec/mpegvideo.c	(revision 8020)
> +++ libavcodec/mpegvideo.c	(working copy)
> @@ -3990,11 +3990,11 @@
>                  if(lowres_flag){
>                      h264_chroma_mc_func *op_pix = s->dsp.put_h264_chroma_pixels_tab;
>  
> -                    if (s->mv_dir & MV_DIR_FORWARD) {
> +                    if ((s->mv_dir & MV_DIR_FORWARD) && s->last_picture.data[0]) {
>                          MPV_motion_lowres(s, dest_y, dest_cb, dest_cr, 0, s->last_picture.data, op_pix);
>                          op_pix = s->dsp.avg_h264_chroma_pixels_tab;
>                      }
> -                    if (s->mv_dir & MV_DIR_BACKWARD) {
> +                    if ((s->mv_dir & MV_DIR_BACKWARD) && s->next_picture.data[0]) {
>                          MPV_motion_lowres(s, dest_y, dest_cb, dest_cr, 1, s->next_picture.data, op_pix);
>                      }
>                  }else{
> @@ -4004,12 +4004,12 @@
>                      }else{
>                          op_pix = s->dsp.put_no_rnd_pixels_tab;
>                      }
> -                    if (s->mv_dir & MV_DIR_FORWARD) {
> +                    if ((s->mv_dir & MV_DIR_FORWARD) && s->last_picture.data[0]) {
>                          MPV_motion(s, dest_y, dest_cb, dest_cr, 0, s->last_picture.data, op_pix, op_qpix);
>                          op_pix = s->dsp.avg_pixels_tab;
>                          op_qpix= s->me.qpel_avg;
>                      }
> -                    if (s->mv_dir & MV_DIR_BACKWARD) {
> +                    if ((s->mv_dir & MV_DIR_BACKWARD) && s->next_picture.data[0]) {
>                          MPV_motion(s, dest_y, dest_cb, dest_cr, 1, s->next_picture.data, op_pix, op_qpix);
>                      }

this change is wrong, the bug is outside MPV_decode_mb(), its not
MPV_decode_mb() job to check the bitstreams validity

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070220/58b3c8b3/attachment.pgp>