[FFmpeg-devel] [PATCH] avutil/imgutils: don't add offsets to NULL pointers

James Almer jamrial at gmail.com
Tue May 4 23:50:36 EEST 2021


On 5/4/2021 5:13 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>   libavutil/imgutils.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
>> index 53faad889a..aaee0dfb7a 100644
>> --- a/libavutil/imgutils.c
>> +++ b/libavutil/imgutils.c
>> @@ -166,7 +166,7 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei
>>       }
>>   
>>       data[0] = ptr;
>> -    for (i = 1; i < 4 && sizes[i]; i++)
>> +    for (i = 1; i < 4 && data[i - 1] && sizes[i]; i++)
>>           data[i] = data[i - 1] + sizes[i - 1];
>>   
>>       return ret;
>> I see two ways to make this a NULL + offset: First, if ptr == NULL; and
> second if data[i - 1] + sizes[i - 1] no longer fits into the allocated
> buffer and happens to yield NULL (very unlikely, but possible) in which
> case data[i] + sizes[i] would be NULL + offset. In the second case, the
> first addition is already undefined behaviour against which we cannot
> guard at all: We don't know the size of the buffer. The only thing we
> can guard against is ptr being NULL; we can even error out in this
> scenario, but I don't know how disruptive that would be.

That'd be an undesirable breakage, yes. Aside from filling data[], the 
function also returns the size of the buffer that should be allocated, 
so that functionality should remain even when ptr == NULL.

> Notice that in C the result of pointer + offset can never be NULL, so a
> compiler could optimize the check for data[i - 1] to just a check for ptr.

If you say there's no warranty that an scenario where data[i-1] + 
size[i-1] == NULL will break the for loop in the next iteration, and no 
way to guard against it at all, then we can just return right before 
attempting to set data[] when ptr == NULL, and at least simplify that 
scenario.

> 
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 



More information about the ffmpeg-devel mailing list