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

[Stefano Sabatini]

On date Friday 2010-11-19 16:06:48 +0100, Stefano Sabatini encoded:
> 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.

Applied.
-- 
FFmpeg = Fiendish and Fostering MultiPurpose Ecumenical Gymnast