| Message ID | 20191214174325.21975-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Headers | show |
On 12/14/2019 2:43 PM, Michael Niedermayer wrote: > Fixes: assertion failure > Fixes: 19301/clusterfuzz-testcase-minimized-ffmpeg_BSF_AV1_FRAME_MERGE_fuzzer-5743212006473728 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/cbs_av1_syntax_template.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c > index 523f9359e4..01c180e4af 100644 > --- a/libavcodec/cbs_av1_syntax_template.c > +++ b/libavcodec/cbs_av1_syntax_template.c > @@ -1601,8 +1601,6 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw, > else > HEADER("Frame Header"); > > - priv->seen_frame_header = 1; > - > #ifdef READ > start_pos = get_bits_count(rw); > #else > @@ -1611,6 +1609,8 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw, > > CHECK(FUNC(uncompressed_header)(ctx, rw, current)); > > + priv->seen_frame_header = 1; > + > if (current->show_existing_frame) { > priv->seen_frame_header = 0; > } else { What's the assertion that fails? av_assert0(priv->frame_header_ref && priv->frame_header)? If so, then if priv->frame_header_ref fails to be allocated and the error is ignored, the same assertion could trigger as seen_frame_header will be 1 even after this patch. I think you can just remove the above line instead of moving it. Right below the call to uncompressed_header() it's going to be set to either 0 or 1 anyway. Another option could be replacing the assertion for a simple check for priv->frame_header_ref that returns invalid data if it's not set.
On Sat, Dec 14, 2019 at 03:21:57PM -0300, James Almer wrote: > On 12/14/2019 2:43 PM, Michael Niedermayer wrote: > > Fixes: assertion failure > > Fixes: 19301/clusterfuzz-testcase-minimized-ffmpeg_BSF_AV1_FRAME_MERGE_fuzzer-5743212006473728 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/cbs_av1_syntax_template.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c > > index 523f9359e4..01c180e4af 100644 > > --- a/libavcodec/cbs_av1_syntax_template.c > > +++ b/libavcodec/cbs_av1_syntax_template.c > > @@ -1601,8 +1601,6 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw, > > else > > HEADER("Frame Header"); > > > > - priv->seen_frame_header = 1; > > - > > #ifdef READ > > start_pos = get_bits_count(rw); > > #else > > @@ -1611,6 +1609,8 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw, > > > > CHECK(FUNC(uncompressed_header)(ctx, rw, current)); > > > > + priv->seen_frame_header = 1; > > + > > if (current->show_existing_frame) { > > priv->seen_frame_header = 0; > > } else { > > What's the assertion that fails? av_assert0(priv->frame_header_ref && > priv->frame_header)? If so, then if priv->frame_header_ref fails to be > allocated and the error is ignored, the same assertion could trigger as > seen_frame_header will be 1 even after this patch. > > I think you can just remove the above line instead of moving it. Right > below the call to uncompressed_header() it's going to be set to either 0 > or 1 anyway. will apply with this suggestion thx > Another option could be replacing the assertion for a simple check for > priv->frame_header_ref that returns invalid data if it's not set. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c index 523f9359e4..01c180e4af 100644 --- a/libavcodec/cbs_av1_syntax_template.c +++ b/libavcodec/cbs_av1_syntax_template.c @@ -1601,8 +1601,6 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw, else HEADER("Frame Header"); - priv->seen_frame_header = 1; - #ifdef READ start_pos = get_bits_count(rw); #else @@ -1611,6 +1609,8 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw, CHECK(FUNC(uncompressed_header)(ctx, rw, current)); + priv->seen_frame_header = 1; + if (current->show_existing_frame) { priv->seen_frame_header = 0; } else {
Fixes: assertion failure Fixes: 19301/clusterfuzz-testcase-minimized-ffmpeg_BSF_AV1_FRAME_MERGE_fuzzer-5743212006473728 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/cbs_av1_syntax_template.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)