[FFmpeg-devel] [PATCH 2/2] avutil/buffer: reject NULL as argument for the av_buffer_pool_init2() alloc callback

[Nicolas George]

James Almer (12020-05-30):
> This prevents NULL pointer dereference crashes when calling av_buffer_pool_get()
> using the resulting pool.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavutil/buffer.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/buffer.c b/libavutil/buffer.c
> index 6d9cb7428e..6fe8f19c39 100644
> --- a/libavutil/buffer.c
> +++ b/libavutil/buffer.c
> @@ -220,7 +220,11 @@ AVBufferPool *av_buffer_pool_init2(int size, void *opaque,
>                                     AVBufferRef* (*alloc)(void *opaque, int size),
>                                     void (*pool_free)(void *opaque))
>  {
> -    AVBufferPool *pool = av_mallocz(sizeof(*pool));
> +    AVBufferPool *pool;
> +
> +    if (!alloc)
> +        return NULL;
> +    pool = av_mallocz(sizeof(*pool));
>      if (!pool)
>          return NULL;

I do not like this: this function can return NULL for AVERROR(ENOMEM),
but now it also means "idiot programmer thought NULL was a valid
callback".

The error code to "idiot programmer did something stupid and should have
read the doc" should be SIGABORT. Proper error return should be reserved
for cases that cannot be tested statically.

So, in this case:

	av_assert0(alloc);

If the code is tested, it is perfectly equivalent anyway, because alloc
will not be NULL.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200531/d33524e3/attachment.sig>