[FFmpeg-devel] [PATCH 7/7] Re-implement avpicture_layout() using pixdesc and imgutils API.

[Stefano Sabatini]

On date Friday 2010-11-19 02:14:16 +0100, Michael Niedermayer encoded:
> On Fri, Nov 19, 2010 at 01:14:24AM +0100, Stefano Sabatini wrote:
> > On date Monday 2010-11-15 14:39:56 +0100, Michael Niedermayer encoded:
> > > On Mon, Nov 15, 2010 at 12:40:43PM +0100, Stefano Sabatini wrote:
> > > > On date Tuesday 2010-11-09 19:31:16 +0100, Michael Niedermayer encoded:
[...]
> > > > that is the linesizes[i] computed by av_fill_image_linesizes() is
> > > > (almost) the same computed by the old code, but the new code is more
> > > > complete and more correct (e.g. with yuyv, uyvy, uyyvyy411, nv12,
> > > > nv21).
> > > 
> > > you copy linesize bytes, you must only copy width*size of sample bytes.
> > 
> > I don't know what you mean, this function computes the linesizes for
> > dest and these new sizes are supposed to be minimal (while the
> > AVPicture in src could contain longer linesizes, but that doesn't
> > matter as the linesizes[] are computed for the *new* frame in dest).
> 
> The issue iam seeing is that the source picture contains something between
> width and linesize and you copy this to destination.

Honestly I can see how this can happen.

        s = src->data[i];
        for (j = 0; j < h; j++) {
            memcpy(dest, s, linesizes[i]);
            dest += linesizes[i];
            s += src->linesize[i];
        }

only linesizes[i] are copied to dest, and linesizes[i] !=
src->linesize[i], and linesizes[i] is minimal (that is can only
contain a picture with size WxH).

> Now while i do not have a concrete example of where this could be a real issue
> it could in principle be part of a exploit leaking information of what is
> displayed in the column to the right of the picture. If this is a problem i
> dont know but it does not feel correct to me.
> If fixing this slows it down then i dont mind us leaving it as is as long as
> noone comes up with a real issue ...
> 
> > 
> > That's also the behavior of the old code.
> 
> I know
> 
> 
> > 
> > > > 
> > > > See result of the attached test code:
> > > 
> > > could you show a diff of this or limit output to changed cases?
> > > its practically unreadable as is
> > 
> > Attached.
> 
> looks ok to me

I don't know if this is an OK to the diff or to the patch. In the
doubt I'm going to apply the patch tomorrow.
-- 
FFmpeg = Faithful & Fantastic Muttering Portable Enchanting Geisha