On Mon, Apr 20, 2020 at 07:34:44PM -0300, James Almer wrote: > On 4/20/2020 7:03 PM, Michael Niedermayer wrote: > > The limit is based on hevcdec.c > > Fixes: > > 20854/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_METADATA_fuzzer-5160442882424832 > > Fixes: out of array access > > > > Found-by: continuous fuzzing process > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > --- > > libavcodec/cbs_h265_syntax_template.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/cbs_h265_syntax_template.c > > b/libavcodec/cbs_h265_syntax_template.c > > index 180a045c34..b74b9648c3 100644 > > --- a/libavcodec/cbs_h265_syntax_template.c > > +++ b/libavcodec/cbs_h265_syntax_template.c > > @@ -1367,7 +1367,7 @@ static int > > FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw, > > infer(num_long_term_sps, 0); > > idx_size = 0; > > } > > - ue(num_long_term_pics, 0, HEVC_MAX_LONG_TERM_REF_PICS); > > + ue(num_long_term_pics, 0, > > FFMIN(HEVC_MAX_LONG_TERM_REF_PICS, FF_ARRAY_ELEMS(current->poc_lsb_lt) - > > current->num_long_term_sps)); > > Maybe poc_lsb_lt should also have HEVC_MAX_LONG_TERM_REF_PICS elems > instead of HEVC_MAX_REFS, same as in the hevc decoder.
ok, btw the decoder and cbs use completely unrelated variable names. That should be cleaned up by someone i think ... > > Also the spec defines some specific limit to this field: > > "When nuh_layer_id is equal to 0, the value of num_long_term_pics shall > be less than or equal to sps_max_dec_pic_buffering_minus1[TemporalId] − > NumNegativePics[CurrRpsIdx] − NumPositivePics[CurrRpsIdx] − > num_long_term_sps − TwoVersionsOfCurrDecPicFlag" > > With CurrRpsIdx derived as: > – If short_term_ref_pic_set_sps_flag is equal to 1, CurrRpsIdx is set > equal to short_term_ref_pic_set_idx. > – Otherwise, CurrRpsIdx is set equal to num_short_term_ref_pic_sets. > > And TwoVersionsOfCurrDecPicFlag as: > "TwoVersionsOfCurrDecPicFlag = pps_curr_pic_ref_enabled_flag && > (sample_adaptive_offset_enabled_flag || > !pps_deblocking_filter_disabled_flag || > deblocking_filter_override_enabled_flag) > When sps_max_dec_pic_buffering_minus1[ TemporalId ] is equal to 0, the > value of TwoVersionsOfCurrDecPicFlag shall be equal to 0." > > But i don't know if it's worth adding that many checks. I saw this too when i wrote the original patch, and i remember that it at least felt like these checks would not cover all cases. Maybe ive missed something but if they dont cover all then this would be unrelated as it would neither be sufficient nor helpfull for this bugfix will later apply this with the HEVC_MAX_LONG_TERM_REF_PICS as suggested and backport unless i hear objections before Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus
signature.asc
Description: PGP signature
_______________________________________________ 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".
