[FFmpeg-devel] [PATCH 01/35] lavu/fifo: disallow overly large fifo sizes

[Anton Khirnov]

Quoting Andreas Rheinhardt (2022-01-13 14:59:49)
> Anton Khirnov:
> > The API currently allows creating FIFOs up to
> > - UINT_MAX: av_fifo_alloc(), av_fifo_realloc(), av_fifo_grow()
> > - SIZE_MAX: av_fifo_alloc_array()
> > However the usable limit is determined by
> > - rndx/wndx being uint32_t
> > - av_fifo_[size,space] returning int
> > so no FIFO should be larger than the smallest of
> > - INT_MAX
> > - UINT32_MAX
> > - SIZE_MAX
> > (which should be INT_MAX an all commonly used platforms).
> > Return an error on trying to allocate FIFOs larger than this limit.
> > ---
> >  libavutil/fifo.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavutil/fifo.c b/libavutil/fifo.c
> > index d741bdd395..f2f046b1f3 100644
> > --- a/libavutil/fifo.c
> > +++ b/libavutil/fifo.c
> > @@ -20,14 +20,23 @@
> >   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> >   */
> >  
> > +#include <stdint.h>
> > +
> >  #include "avassert.h"
> >  #include "common.h"
> >  #include "fifo.h"
> >  
> > +#define FIFO_SIZE_MAX FFMIN3((uint64_t)INT_MAX, (uint64_t)UINT32_MAX, (uint64_t)SIZE_MAX)
> 
> Aren't these casts unnecessary?

Possibly yes. I mainly added them so that FIFO_SIZE_MAX is always the
same type, which might prevent surprises. I can drop the casts if you
think they are never necessary.

> And actually dangerous? (They add the implicit requirement that
> INT_MAX and SIZE_MAX fit into an uint64_t.)

I'll grant you _theoretically_ dangerous - I've never heard of a
posix-compliant platform with sizeof(int) > 64.

And in any case that macro should be gone with the old API (was going to
include a patch scheduling its removal, but apparently forgot - will
send it later).

-- 
Anton Khirnov