On 10/9/2020 6:16 PM, Andreas Rheinhardt wrote: > James Almer: >> 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 <jamr...@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. >> > Your calculation seems to be off by a factor of four because you ignore > that every byte removed needs an entry of type int*.
True, forgot each byte position was saved as an int in the array. > Furthermore 26kB (I > presume you meant kB and not kb) seems very, very small for a normal > file these days; it feels more like 320p streaming or so. Yes, kilobytes. And as i said, it was a 2160p sample where the biggest packet was 26kB. Maybe it's not the most complex stream out there, i don't know. > > Furthermore, there is a bigger issue with your patch: It can lead to > quadratic memory consumption for annex B input: Because the length of > the NAL units is not known in advance, length in the above code refers > to the length from the beginning of the NAL unit to the end of the > packet. So this code will allocate gigantic amounts of memory for a > packet containing lots of small NAL units, each with a single 0x000003. > This is an issue similar to the one fixed by > 03b82b3ab9883cef017e513c7d0b3b986b3b3e7b. Alright, will use nal->skipped_bytes * 2 then. > > - Andreas > > *: Yes, I know there is no guarantee that sizeof(int) is four. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
