[FFmpeg-devel] [PATCH] avcodec/h2645_parse: remove initial skipped_bytes_pos buffer
James Almer
jamrial at gmail.com
Fri Oct 9 23:57:22 EEST 2020
On 10/9/2020 1:36 PM, Michael Niedermayer wrote:
> On Thu, Oct 08, 2020 at 10:25:11AM -0300, James Almer wrote:
>> Allocate it only when needed, and instead of giving it a fixed initial size
>> that's doubled on each realloc, ensure it's always big enough for the NAL
>> currently being parsed.
>>
>> Fixes: OOM
>> Fixes: 23817/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_METADATA_fuzzer-6300869057576960
>>
>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> libavcodec/h2645_parse.c | 28 ++++++++++------------------
>> 1 file changed, 10 insertions(+), 18 deletions(-)
>>
>> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
>> index 0f98b49fbe..f5c76323c1 100644
>> --- a/libavcodec/h2645_parse.c
>> +++ b/libavcodec/h2645_parse.c
>> @@ -108,22 +108,20 @@ int ff_h2645_extract_rbsp(const uint8_t *src, int length,
>> dst[di++] = 0;
>> si += 3;
>>
>> - if (nal->skipped_bytes_pos) {
>> - nal->skipped_bytes++;
>> - if (nal->skipped_bytes_pos_size < nal->skipped_bytes) {
>> - nal->skipped_bytes_pos_size *= 2;
>> - av_assert0(nal->skipped_bytes_pos_size >= nal->skipped_bytes);
>> - av_reallocp_array(&nal->skipped_bytes_pos,
>> + nal->skipped_bytes++;
>> + if (nal->skipped_bytes_pos_size < nal->skipped_bytes) {
>> + nal->skipped_bytes_pos_size = length / 3;
>
> This would allocate a much larger than needed skipped_bytes_pos for
> probably nearly all of the real world h264 files to fix an issue with
> crafted streams.
>
> The initial size should be choosen so as to be large enough for real world
> streams. If that turns out to be too small then length /3 sounds reasonable
> as the first reallocation.
At most 1/3 of the bytes in a NAL would be removed by this code, hence
length / 3. I could make it length / 16 like in your fix if you prefer,
or maybe nal->skipped_bytes * 2 to keep it closer to the current
behavior, but real world samples don't have more than a handful of NALs
per packet, and not all have escaped bytes that need to be removed (If
there are none, nothing will be allocated).
I looked at a 2160p hevc sample, and the biggest packet is about 26kb,
which assuming it's a single slice NAL, it will be about 8kb for the
skipped_bytes_pos buffer with length / 3.
>
> thx
> [...]
>
>
> _______________________________________________
> 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