[MPlayer-dev-eng] [PATCH] Fix uninitialized memory read in vd_ffmpeg.c

[Reimar Döffinger]

On Fri, Mar 11, 2011 at 11:43:54AM +0100, Tobias Diedrich wrote:
> Reimar Döffinger wrote:
> > On Thu, Mar 10, 2011 at 02:25:12AM +0200, Ivan Kalvachev wrote:
> > > On 3/9/11, Tobias Diedrich <ranma at tdiedrich.de> wrote:
> > > > Reimar Döffinger wrote:
> > > >> Required padding is 32 bytes, so this will not use uninitialized data
> > > >> (unless len is 0 and data NULL I guess -  seems like that has never
> > > >> happened
> > > >> so far though).
> > > >> Since in addition this change means you can no longer debug packages
> > > >> shorter
> > > >> than 16 bytes I'm rather against it.
> > > >
> > > > How about this?
> > > >
> > > > Index: mplayer-svn-vanilla/libmpcodecs/vd_ffmpeg.c
> > > > ===================================================================
> > > > --- mplayer-svn-vanilla.orig/libmpcodecs/vd_ffmpeg.c	2011-02-10
> > > > 21:44:56.791660244 +0100
> > > > +++ mplayer-svn-vanilla/libmpcodecs/vd_ffmpeg.c	2011-03-09
> > > > 22:25:17.415539937 +0100
> > > > @@ -828,8 +828,14 @@
> > > >
> > > >      avctx->hurry_up=(flags&3)?((flags&2)?2:1):0;
> > > >
> > > > -    mp_msg(MSGT_DECVIDEO, MSGL_DBG2, "vd_ffmpeg data: %04x, %04x, %04x,
> > > > %04x\n",
> > > > -           ((int *)data)[0], ((int *)data)[1], ((int *)data)[2], ((int
> > > > *)data)[3]);
> > > > +    if (mp_msg_test(MSGT_DECVIDEO, MSGL_DBG2)) {
> > > > +        int buf[4] = { 0, 0, 0, 0 };
> > > > +        memcpy(buf, data, len > sizeof(buf) ? sizeof(buf) : len);
> > > > +        mp_msg(MSGT_DECVIDEO, MSGL_DBG2,
> > > > +               "vd_ffmpeg data: %04x, %04x, %04x, %04x\n",
> > > > +               buf[0], buf[1], buf[2], buf[3]);
> > > > +    }
> > > > +
> > > >      av_init_packet(&pkt);
> > > >      pkt.data = data;
> > > >      pkt.size = len;
> > 
> > Well, I said that this should not use uninitialized data anyway,
> > so the question still remains: why?
> 
> Is it always zero-padded?  If not it's still some random crap we may
> print there.

Who knows. There's a certain chance it is a bug if not though.