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 <[email protected]> >> --- >> 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 > [email protected] > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > [email protected] with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
