[FFmpeg-devel] [PATCH] avcodec/hevcdec: check that the local context list exists before dereferencing it

[Paul B Mahol]

On Wed, Feb 10, 2021 at 6:57 PM Andreas Rheinhardt <
andreas.rheinhardt at gmail.com> wrote:

> James Almer:
> > Since the decoder is not flagged as init cleanup capable,
> hevc_decode_free()
> > is being called manually if the hevc_decode_extradata() call fails at
> the end
> > of hevc_decode_init().
> > In a frame threading scenario, however, if AVCodec->init() returns an
> error,
> > ff_frame_thread_free() will be called regardless of the above flag being
> set
> > or not, resulting in hevc_decode_free() being called a second time for
> the
> > same context.
> >
> > Solve this by ensuring pointers are not dereferenced if they are NULL,
> and
> > setting the decoder as init cleanup capable.
> >
> > Fixes ticket #9099.
> >
> > Signed-off-by: James Almer <jamrial at gmail.com>
> > ---
> > Maybe ff_frame_thread_free() should not call AVCodec->close() for thread
> contexts
> > where AVCodec->init() failed and FF_CODEC_CAP_INIT_CLEANUP is not set?
> >
>
> Fixing this has been on my to-do list. (The situation is even worse than
> you describe it: It is possible that AVCodec->close is called on an
> AVCodecContext whose private_data couldn't be allocated.)
>

So how should proceed? Apply this patch and fix other issues after it?

>
> >  libavcodec/hevcdec.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> > index 92eb888033..898dac8cbb 100644
> > --- a/libavcodec/hevcdec.c
> > +++ b/libavcodec/hevcdec.c
> > @@ -3417,6 +3417,7 @@ static av_cold int hevc_decode_free(AVCodecContext
> *avctx)
> >      av_freep(&s->sh.offset);
> >      av_freep(&s->sh.size);
> >
> > +    if (s->HEVClcList && s->sList) {
> >      for (i = 1; i < s->threads_number; i++) {
> >          HEVCLocalContext *lc = s->HEVClcList[i];
> >          if (lc) {
> > @@ -3424,9 +3425,8 @@ static av_cold int hevc_decode_free(AVCodecContext
> *avctx)
> >              av_freep(&s->sList[i]);
> >          }
> >      }
> > -    if (s->HEVClc == s->HEVClcList[0])
> > -        s->HEVClc = NULL;
> > -    av_freep(&s->HEVClcList[0]);
> > +    }
> > +    av_freep(&s->HEVClc);
> >      av_freep(&s->HEVClcList);
> >      av_freep(&s->sList);
> >
> > @@ -3622,7 +3622,6 @@ static av_cold int hevc_decode_init(AVCodecContext
> *avctx)
> >          if (avctx->extradata_size > 0 && avctx->extradata) {
> >              ret = hevc_decode_extradata(s, avctx->extradata,
> avctx->extradata_size, 1);
> >              if (ret < 0) {
> > -                hevc_decode_free(avctx);
> >                  return ret;
> >              }
> >          }
> > @@ -3673,7 +3672,7 @@ AVCodec ff_hevc_decoder = {
> >      .capabilities          = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
> >                               AV_CODEC_CAP_SLICE_THREADS |
> AV_CODEC_CAP_FRAME_THREADS,
> >      .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE |
> FF_CODEC_CAP_EXPORTS_CROPPING |
> > -                             FF_CODEC_CAP_ALLOCATE_PROGRESS,
> > +                             FF_CODEC_CAP_ALLOCATE_PROGRESS |
> FF_CODEC_CAP_INIT_CLEANUP,
> >      .profiles              = NULL_IF_CONFIG_SMALL(ff_hevc_profiles),
> >      .hw_configs            = (const AVCodecHWConfigInternal *const []) {
> >  #if CONFIG_HEVC_DXVA2_HWACCEL
> >
>
> _______________________________________________
> 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".