[FFmpeg-devel] Overflow check for frame_size in v4l.c

Stefano Sabatini stefano.sabatini-lala
Sun Dec 28 01:25:36 CET 2008


On date Saturday 2008-12-27 23:33:19 +0100, Michael Niedermayer encoded:
> On Sat, Dec 27, 2008 at 06:01:37PM +0100, Stefano Sabatini wrote:
> > On date Saturday 2008-12-27 17:16:05 +0100, Michael Niedermayer encoded:
> > > On Sat, Dec 27, 2008 at 03:12:59PM +0100, Stefano Sabatini wrote:
> > > > On date Saturday 2008-12-27 14:36:37 +0100, Michael Niedermayer encoded:
> > [...]
> > > > > > > I think the check is insufficient and more not less checking is needed
> > > > > > > 
> > > > > > >  frame_size = s->video_win.width * s->video_win.height * video_formats[j].depth / 8;
> > > > > > > 
> > > > > > > will not work with 32767*32767*...
> > > > > > 
> > > > > > OK, 32767 = 2^15 -1.
> > > > > > 
> > > > > > We may then check for 16383 = 2^14 -1 (check the patch below), or
> > > > > > maybe some function like these ones may help:
> > > > > 
> > > > > avcodec_check_dimensions()
> > > > 
> > > > I don't think it is a good idea to use that function here, its domain
> > > 
> > > its exactly what the function is designed to check.
> > 
> > /**
> >  * Checks if the given dimension of a picture is valid, meaning that all
> >  * bytes of the picture can be addressed with a signed int.
> >  *
> >  * @param[in] w Width of the picture.
> >  * @param[in] h Height of the picture.
> >  * @return Zero if valid, a negative value if invalid.
> >  */
> > int avcodec_check_dimensions(void *av_log_ctx, unsigned int w, unsigned int h);
> > 
> > The definition is promising, yet I'm missing the part which deals with
> > the pixel depth.
> > 
> > Then reading the code my confusion increased, as I can't figure out
> > the meaning of the code (what I -- naively -- expected was something like:
> > w * h < (INT_MAX -1 ) / MAX_DEPTH) (I guess those 128s are for

uh, I meant w * h < INT_MAX / MAX_DEPTH

> > alignment reasons?):
> 
> some codecs work with fixed sized blocks like the 16x16 macroblocks ...
> i guess the 128 and 4 should be replaced by "named constants"

My only problem with the use of this function is that it forces checks
stricter than the necessary on most occasions, indeed not always depth
is 4 and not only the fixed sized block considerations applied.

Anyway new try attached.

Note that in the v4l case we need to move the check *after* the size
is eventually automatically set (furthermore if w/h is zero then
avcodec_check_dimensions() will fail).

[...]

Regards.
-- 
FFmpeg = Forgiving & Friendly Muttering Portable Energized Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v4l12-safer-size-check.patch
Type: text/x-diff
Size: 1445 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081228/8ae75740/attachment.patch>



More information about the ffmpeg-devel mailing list