[FFmpeg-devel] [PATCH v6] avcodec: add farbfeld encoder
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Wed Jun 5 14:14:01 EEST 2024
Stefano Sabatini:
> On date Wednesday 2024-06-05 12:02:08 +0200, Andreas Rheinhardt wrote:
>> Stefano Sabatini:
>>> On date Tuesday 2024-06-04 17:28:35 -0500, Marcus B Spencer wrote:
> [...]
>>>> +#define HEADER_SIZE 16
>>>> +
>>>> +static int farbfeld_encode_frame(AVCodecContext *ctx, AVPacket *pkt,
>>>> + const AVFrame *p, int *got_packet)
>>>> +{
>>>
>>>> + int raw_img_size = av_image_get_buffer_size(
>>>> + p->format,
>>>> + p->width,
>>>> + p->height,
>>>> + 1
>>>> + );
>>>
>>>> + int64_t buf_size = (int64_t)raw_img_size + HEADER_SIZE;
>>>
>>> not yet, this might change the sign for a negative raw_img_size, you
>>> need two separate checks (one is not enough), as in the following:
>>>
>>> int raw_img_size = av_image_get_buffer_size(p->format, p->width,p->height, 1);
>>>
>>> if (raw_image_size < 0)
>>> return raw_image_size;
>>>
>>> int buf_size = raw_img_size + HEADER_SIZE;
>>> if (buf_size < 0)
>>> return AVERROR(EINVAL);
>>
>
>> This is absolutely wrong: raw_img_size is nonnegative here as is
>> HEADER_SIZE and the addition will be performed as an int, so that
>> overflow would be UB which implies that the compiler can optimize this
>> check away.
>
> Correct, the following should avoid the UB if I'm not mistaken again:
>
> if (HEADER_SIZE > (INT_MAX - raw_img_size))
> return AVERROR(EINVAL);
> int buf_size = raw_img_size + HEADER_SIZE;
> ...
>
>> One does not need two checks as long as int is 32 bits (because then one
>> can just perform the addition in 64bits).
>
> sizeof(int) is not defined by the C standard, so you cannot assume it
> is 32 bits (even if on most platforms/compilers it will be)
>
Did you even read the following? It handles the case where simply using
64bits is not enough.
>> Just use the following (#if
>> has been used because compilers have a tendency to emit warnings if a
>> particular check is tautologically false):
>>
>> #if INT_MAX > INT64_MAX - HEADER_SIZE
>> if (raw_img_size > INT64_MAX - HEADER_SIZE)
>> return AVERROR(ERANGE);
>> #endif
More information about the ffmpeg-devel
mailing list