[vlc-devel] [PATCH] input: decoder: fix NULL deref on early cancellation

[Steve Lhomme]

The patch won't hurt but I also wonder how it's possible to end up in 
this situation.

The decoder thread is not canceled as far as I can see in decoder.c. So 
that means the ModuleThread in this case is an internal thread of the 
decoder (likely lavc). If it's canceled it should prevent other threads 
trying to continue using the decoder after that. In this case the 
"decoder" object is a resource that should be protected from multiple 
thread accesses at the same time.

So maybe this fix (if it actually happened) is hiding a different problem.

On 2020-03-10 7:49, Thomas Guillem wrote:
> 
> 
> On Mon, Mar 9, 2020, at 22:50, Francois Cartegnie wrote:
>> ---
>>   src/input/decoder.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/input/decoder.c b/src/input/decoder.c
>> index d868c21f23..998a2816bb 100644
>> --- a/src/input/decoder.c
>> +++ b/src/input/decoder.c
>> @@ -650,7 +650,8 @@ static picture_t *ModuleThread_NewVideoBuffer(
>> decoder_t *p_dec )
>>   {
>>       struct decoder_owner *p_owner = dec_get_owner( p_dec );
>>       assert( p_owner->p_vout );
>> -
>> +    if( p_owner->out_pool == NULL )
>> +        return NULL;
> 
> 
> The big question is : why is the out_pool destroyed before the decoder module in DeleteDecoder ?

It makes sense to remove the pictures allocated by the decoder before 
the decoder module is gone and possibly DLLs not accessible anymore to 
do it (DVXA.DLL for example).

In practice this pool is a shadow of the actual surfaces allocated by 
the decoder in GPU decoding. It may not hold any "context" at this 
point. It depends if some pictures are in-flight (sent by the decoder to 
be used) or not. In CPU decoding it's not an issue. This pool could also 
go away if each decoder managed its pool but that is not the case yet.

> So that every future decoder_NewPicture fails ? In that case, this patch is OK for me.
> 
> Steve ?
> 
>>       picture_t *pic = picture_pool_Wait( p_owner->out_pool );
>>       if (pic)
>>           picture_Reset( pic );
>> -- 
>> 2.24.1
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>