[FFmpeg-devel] [PATCH 2/3] avformat/evcdec: Check that enough data has been read

[James Almer]

On 7/6/2023 10:30 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 7/6/2023 10:14 PM, James Almer wrote:
>>> On 7/6/2023 6:08 PM, Andreas Rheinhardt wrote:
>>>> Fixes potential use of uninitialized values
>>>> in evc_read_nal_unit_length().
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>>> ---
>>>>    libavformat/evcdec.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
>>>> index 9886542311..0f464930f7 100644
>>>> --- a/libavformat/evcdec.c
>>>> +++ b/libavformat/evcdec.c
>>>> @@ -162,6 +162,8 @@ static int evc_read_packet(AVFormatContext *s,
>>>> AVPacket *pkt)
>>>>            ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>>>>            if (ret < 0)
>>>>                return ret;
>>>> +        if (ret != EVC_NALU_LENGTH_PREFIX_SIZE)
>>>> +            return AVERROR_INVALIDDATA;
>>>
>>> There's a ffio_ensure_seekback() for EVC_NALU_LENGTH_PREFIX_SIZE bytes
>>> immediately before the avio_read() call. Shouldn't that be enough to
>>> guarantee that much can be read?
>>>
>>> Also, you can just pass ret to evc_read_nal_unit_length() below
>>> instead of adding this check here. It will return an error if it's <
>>> EVC_NALU_LENGTH_PREFIX_SIZE.
>>
>> Oh, my bad, i was looking at the function of the same name in
>> libavcodec/evc_parse.h
>>
>> The function in evc.h could be changed to also use the same check as the
>> one the evc_parse.h version alongside the other change you're doing in
>> patch 3/3.
>>
> 
> These functions already do the same (except for the log message); they
> both return 0 if not enough data is available. The return value would
> need to be int64_t if one wanted to return both error codes and lengths
> via it. I don't really see the advantage of this.

Fair enough. Then what about ffio_ensure_seekback()? Should that not 
guarantee at least EVC_NALU_LENGTH_PREFIX_SIZE bytes are buffered, thus 
readable and seekable backwards?