[FFmpeg-devel] [PATCH] avcodec/vp9: avoid using uninitialized mutex/condition

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Thu Sep 2 12:27:38 EEST 2021


Steve Lhomme:
> When using slice decoding vp9_free_entries is called before vp9_alloc_entries
> is ever called. It should destroy properly initialized variables (or check it
> was never called before).
> 
> It usually works undetected as pthread implementations allows NULL as a special
> value (and should return EINVAL but doesn't). But pthreadGC2 doesn't allow NULL
> in pthread_mutex_destroy() and crashes when that's the case.
> ---
>  libavcodec/vp9.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> index 874005a5ae..d40e90b70e 100644
> --- a/libavcodec/vp9.c
> +++ b/libavcodec/vp9.c
> @@ -1796,6 +1796,10 @@ static av_cold int vp9_decode_init(AVCodecContext *avctx)
>  
>      s->last_bpp = 0;
>      s->s.h.filter.sharpness = -1;
> +    if (avctx->active_thread_type & FF_THREAD_SLICE)  {
> +        pthread_mutex_init(&s->progress_mutex, NULL);
> +        pthread_cond_init(&s->progress_cond, NULL);
> +    }
>  
>      for (int i = 0; i < 3; i++) {
>          s->s.frames[i].tf.f = av_frame_alloc();
> 
1. progress_mutex and progress_cond don't exist if compiled without
threading support, so your patch will lead to compilation failures in
this case.
2. I don't see a reason why these mutexes need to be initialized and
destroyed when the dimensions/number of tile cols change at all. They
should be initialized once during init (if slice threading is used) and
freed during close (if they have been successfully initialized).
3. Initializing this condition and mutex is currently not checked.

I can fix this if you want.

- Andreas


More information about the ffmpeg-devel mailing list