[FFmpeg-devel] [PATCH] too late bounds check in mpeg1_decode_block_intra

[Michael Niedermayer]

On Thu, Apr 10, 2008 at 06:40:04PM +0200, Reimar D?ffinger wrote:
> On Thu, Apr 10, 2008 at 06:26:25PM +0200, Michael Niedermayer wrote:
> > On Thu, Apr 10, 2008 at 05:35:03PM +0200, Reimar D?ffinger wrote:
> > > Hello,
> > > someone on MPlayer-users seems to have problems due to this. While I
> > > can't test myself, the code seems obviously wrong to me.
> > > Attached is my suggestion to fix it, though there are other
> > > possibilities, like increasing the size of intra_scantable.permutated
> > > to 256 and maybe more.
> > 
> > First i dont see a problem and would thus like to see how exactly something
> > can go wrong. there are 64 entries (raster_end) after "permutated" and i dont
> > see how the code could read past them. Its maybe not beautifull but as long
> > as there is no bug iam against making to code more complex.
> 
> I just assumed that the raster_end entries might be > 63. 

They are build by
    end=-1;
    for(i=0; i<64; i++){
        int j;
        j = st->permutated[i];
        if(j>end) end=j;
        st->raster_end[i]= end;
    }
Thus they should not be larger than permutated



> And those fields
> maybe should have an additional comment then if we decide that the code
> should be allowed to assume their order.

Well, yes .... iam ok with you adding one ...


> The only additional information I have so far is that it only happens
> with MPlayer playing from /dev/video0, that it does not happen with a
> file obtained via -dumpstream and this backtrace:

sounds slightly suspicious.


> 
> 0x084404c6 in mpeg_decode_mb (s=0x88eb020, block=<value optimized out>) at mpeg12.c:1162
> 1162                    level= (level*qscale*quant_matrix[j])>>4;

disassembly, and the values of j and i would be interresting as well.
And of course the compiler version :)


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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080410/48e62e06/attachment.pgp>