[vlc-devel] [PATCH] decoder: fix out_pool NULL deref

[Steve Lhomme]

On 2020-03-10 8:56, Thomas Guillem wrote:
> Once the format is configured, a decoder module can request new picture_t via
> decoder_NewPicture() asynchronously. Therefore, the out_pool need to outlive
> the decoder module.

If your decoder is asking for a new picture_t when it's actually being 
destroyed, you probably a synchronization problem.

You can use your own pool in this asynchronous decoder and never call 
decoder_NewPicture() at all. You can then handle this synchronization 
locally to make sure you don't try to get pictures when your decoder is 
being killed. IMO that should be the way to go now that we have push.

That being said there's no way for the decoder to know it's being killed 
until decoder_Clean() is called. So I think this patch is OK.

There could be an issue with resources allocated by the module still 
held in the pool. Releasing them after the module is destroyed could 
break things. But that's something we already have to manage with 
in-flight pictures where the resources are released with the last 
picture of the pool. All the modules using va_surface are clean with 
that, and AFAIK the other one or two that don't are also clean.

> This patch fixes a NULL deref from decoder_NewPicture() when the decoder is
> being destroyed. Indeed, the module need to be unloaded before the out_pool is
> destroyed.
> ---
>   src/input/decoder.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index d868c21f23..7dbc2b8f8d 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -1962,13 +1962,13 @@ static void DeleteDecoder( decoder_t * p_dec )
>       msg_Dbg( p_dec, "killing decoder fourcc `%4.4s'",
>                (char*)&p_dec->fmt_in.i_codec );
>   
> +    decoder_Clean( p_dec );
>       const enum es_format_category_e i_cat =p_dec->fmt_in.i_cat;
>       if ( p_owner->out_pool )
>       {
>           picture_pool_Release( p_owner->out_pool );
>           p_owner->out_pool = NULL;
>       }
> -    decoder_Clean( p_dec );
>   
>       if (p_owner->vctx)
>           vlc_video_context_Release( p_owner->vctx );
> -- 
> 2.20.1
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>