On 5/31/2020 7:30 AM, Nicolas George wrote:
> 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 <jamr...@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,

I guess replacing one crash with another earlier crash is not too bad
(Although i dislike asserts used to catch invalid arguments), but we
can't chalk it up to "idiot user that didn't read the docs" seeing how
in some functions certain parameters may be NULL and it's undocumented
(see patch 1/2 from this same set).

An alternative is doing the same as av_buffer_pool_init() and falling
back to using the default allocator if none is passed, but that's a
change in behavior (From not working to working, instead of from
crashing to crashing earlier).
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to