[FFmpeg-devel] [FFmpeg-cvslog] avcodec/avpacket: Avoid unspecific return -1 for av_grow_packet()

[Carl Eugen Hoyos]

2019-01-06 14:43 GMT+01:00, James Almer <jamrial at gmail.com>:
> On 1/6/2019 10:36 AM, Carl Eugen Hoyos wrote:
>> 2019-01-02 0:23 GMT+01:00, Michael Niedermayer <git at videolan.org>:
>>> ffmpeg | branch: master | Michael Niedermayer <michael at niedermayer.cc> |
>>> Mon
>>> Dec 31 18:25:18 2018 +0100| [9520d51e21f9aa5adc807b0b89322bd822b06738] |
>>> committer: Michael Niedermayer
>>>
>>> avcodec/avpacket: Avoid unspecific return -1 for av_grow_packet()
>>>
>>> Reviewed-by: Paul B Mahol <onemda at gmail.com>
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>>
>>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=9520d51e21f9aa5adc807b0b89322bd822b06738
>>> ---
>>>
>>>  libavcodec/avpacket.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>>> index e160ad3033..11ac4e80cd 100644
>>> --- a/libavcodec/avpacket.c
>>> +++ b/libavcodec/avpacket.c
>>> @@ -112,7 +112,7 @@ int av_grow_packet(AVPacket *pkt, int grow_by)
>>>      av_assert0((unsigned)pkt->size <= INT_MAX -
>>> AV_INPUT_BUFFER_PADDING_SIZE);
>>>      if ((unsigned)grow_by >
>>>          INT_MAX - (pkt->size + AV_INPUT_BUFFER_PADDING_SIZE))
>>> -        return -1;
>>> +        return AVERROR(ENOMEM);
>>>
>>>      new_size = pkt->size + grow_by + AV_INPUT_BUFFER_PADDING_SIZE;
>>>      if (pkt->buf) {
>>> @@ -124,7 +124,7 @@ int av_grow_packet(AVPacket *pkt, int grow_by)
>>>          } else {
>>>              data_offset = pkt->data - pkt->buf->data;
>>>              if (data_offset > INT_MAX - new_size)
>>> -                return -1;
>>> +                return AVERROR(ENOMEM);
>>
>> Is this really correct?
>> At least on some 64bit systems, larger allocations should be possible,
>> we are simply assuming invalid data if such an allocation is tried, no?
>
> Even if av_max_alloc() allows the API user to allocate more than INT_MAX
> bytes, AVPackets can't handle it as they use int for size.

That's what I am saying: There is no memory issue in above case,
just that we refuse to handle the case.

Carl Eugen