[FFmpeg-devel] [PATCH 10/10] Add av_image_alloc() and use it in avfilter_default_get_video_buffer().

[Stefano Sabatini]

On date Friday 2010-11-26 17:08:57 +0100, Stefano Sabatini encoded:
> On date Tuesday 2010-11-16 05:42:28 +0100, Michael Niedermayer encoded:
> > On Mon, Nov 15, 2010 at 12:05:48PM +0100, Stefano Sabatini wrote:
> > > On date Sunday 2010-11-14 20:22:06 +0100, Michael Niedermayer encoded:
> > > > On Sun, Nov 14, 2010 at 12:26:40PM +0100, Stefano Sabatini wrote:
> > > [...]
> > > > > > > +int av_image_alloc(uint8_t *pointers[4], int linesizes[4],
> > > > > > > +                   int w, int h, enum PixelFormat pix_fmt, int align)
> > > > > > > +{
> > > > > > > +    int i, ret;
> > > > > > > +    uint8_t *buf;
> > > > > > > +
> > > > > > > +    if ((ret = av_image_fill_linesizes(linesizes, pix_fmt, w)) < 0)
> > > > > > > +        return ret;
> > > > > > > +
> > > > > > > +    for (i = 0; i < 4; i++)
> > > > > > > +        linesizes[i] = FFALIGN(linesizes[i], align);
> > > > > > > +
> > > > > > > +    if ((ret = av_image_fill_pointers(pointers, pix_fmt, h, NULL, linesizes)) < 0)
> > > > > > > +        return ret;
> > > > > > 
> > > > > > av_image_fill_pointers() does not seem to check w/h being valid and thus may
> > > > > > be exploitable
> > > > > 
> > > > > Added av_image_check_size() check, I don't know if we should add to
> > > > > the function params a log_offset and log_ctx, or simply disable
> > > > > logging in all cases.
> > > > > 
> > > > > Also I see a function:
> > > > > int av_image_get_size(int linesizes[4], int w, int h, enum PixFmt pix_fmt, int align);
> > > > > 
> > > > > may be useful as well.
> > > > > -- 
> > > > > FFmpeg = Fascinating and Faithless Moronic Plastic Excellent Ghost
> > > > 
> > > > >  libavcore/imgutils.c   |   29 +++++++++++++++++++++++++++++
> > > > >  libavcore/imgutils.h   |   11 +++++++++++
> > > > >  libavfilter/defaults.c |   17 +++--------------
> > > > >  3 files changed, 43 insertions(+), 14 deletions(-)
> > > > > 7b6e0f3d7edb8b14b71429036913e6574b187fa8  0002-Implement-av_image_alloc-and-use-it-in.patch
> > > > > From 7f62492ee5a9ce930b37fd2f6d6e6d2dde2d8ccb Mon Sep 17 00:00:00 2001
> > > > > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > > > > Date: Sun, 7 Nov 2010 16:51:50 +0100
> > > > > Subject: [PATCH 2/7] Implement av_image_alloc() and use it in
> > > > >  avfilter_default_get_video_buffer().
> > > > > 
> > > > > ---
> > > > >  libavcore/imgutils.c   |   29 +++++++++++++++++++++++++++++
> > > > >  libavcore/imgutils.h   |   11 +++++++++++
> > > > >  libavfilter/defaults.c |   17 +++--------------
> > > > >  3 files changed, 43 insertions(+), 14 deletions(-)
> > > > > 
> > > > > diff --git a/libavcore/imgutils.c b/libavcore/imgutils.c
> > > > > index 554639f..3f7df7a 100644
> > > > > --- a/libavcore/imgutils.c
> > > > > +++ b/libavcore/imgutils.c
> > > > > @@ -161,6 +161,35 @@ int ff_set_systematic_pal2(uint32_t pal[256], enum PixelFormat pix_fmt)
> > > > >      return 0;
> > > > >  }
> > > > >  
> > > > > +int av_image_alloc(uint8_t *pointers[4], int linesizes[4],
> > > > > +                   int w, int h, enum PixelFormat pix_fmt, int align)
> > > > > +{
> > > > > +    int i, ret;
> > > > > +    uint8_t *buf;
> > > > > +
> > > > > +    if ((ret = av_image_check_size(w, h, 0, NULL)) < 0)
> > > > > +        return ret;
> > > > 
> > > > thats the wrong place to check,
> > > > av_image_fill_linesizes and av_image_fill_pointers still will fail if they are
> > > > used with too large w/h i think
> > > 
> > > AFAIK the checks in av_image_check_size() should be enough, they're
> > > even stricter than necessary for most pixel formats.
> > 
> > you can even add a return 0 here its still not going to fix
> > the lack of a check in av_image_fill_pointers()
> 
> Updated. I sent a separate patch for the missing checks.

I'll apply in three days if there are no objections.
-- 
FFmpeg = Fierce & Frightening Muttering Puristic Elastic God