[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

[Nicolas George]

Juan De León (12019-08-19):
> > > +    size_t size = sizeof(AVEncodeInfoFrame) +
> > sizeof(AVEncodeInfoBlock)*FFMAX((int)(nb_blocks - 1), 0);
> >
> > (Commenting from my phone because urgent.)
> > This line do not make sense to me. Can you explain the reason for the
> > cast and how you avoid overflows?
> >
> In AVFrameEncodeInfo the block array is initialized as blocks[1], so when
> allocating memory for the array we must consider 1 block is already
> allocated.
> nb_blocks can be 0, if it is unsigned the subtraction wraps it around to
> UINT_MAX, the cast to int is to prevent that.

The cast to int does not prevent the wrap, because the wrap happens
before the cast. All the cast does is cause an undefined behaviour,
which will in practice yield -1, but then you multiply by a sizeof,
which is unsigned.

> If this is confusing, I think it's better to change it to check for
> nb_blocks > 0 and subtract 1.

Indeed, it is the simplest clean solution. You can do that by swapping
the FFMAX and the subtraction. As it is, the FFMAX does nothing anyway.

Or you can dispense with the FFMAX entirey: sizeof(info) - sizeof(block)
+ n * sizeof(block). It result in allocating an incomplete info
structure, which I think is fine because nothing will access the missing
part.

In any case, you need to check that neither the multiplication nor the
division overflow. And you need to do it beforehand.

Regards,

-- 
  Nicolas George