[vlc-devel] [PATCH] nvdec: fix segfault in nvdec_gl

[Quentin Chateau]

interop->priv is set only at the end of the Open function. Which means 
it will be NULL in the earlier call to Close.

I agree that in this case there is nothing to do in Close and we could 
just remove it. Though I made the assumption this call to Close was here 
to prepare further changes. I usually don't use nvdec_gl, but I ran 
across this bug and thought I could send the minimal patch to fix the issue.

On 06/04/2020 09:41, Steve Lhomme wrote:
> Also the only place Close is called from Open, p_sys cannot be NULL. 
> So maybe it's related to some local code you have ?
>
> On 2020-04-06 9:21, Steve Lhomme wrote:
>> Given it's doing just one thing it's cleaner not to call Close at all 
>> and just do the cleanup in Open.
>>
>> On 2020-04-06 8:58, Quentin Chateau wrote:
>>> It happens when Close is called from the Open function in an error path
>>> On Apr 6, 2020, at 08:56, Steve Lhomme <robux4 at ycbcr.xyz 
>>> <mailto:robux4 at ycbcr.xyz>> wrote:
>>>
>>>     On 2020-04-03 17:44, quentin.chateau at deepskycorp.com wrote:
>>>
>>>         From: Quentin Chateau <quentin.chateau at deepskycorp.com>
>>>
>>>         ---
>>>         modules/hw/nvdec/nvdec_gl.c | 3 ++-
>>>         1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>>         diff --git a/modules/hw/nvdec/nvdec_gl.c
>>>         b/modules/hw/nvdec/nvdec_gl.c
>>>         index 137e730028..18a34d061c 100644
>>>         --- a/modules/hw/nvdec/nvdec_gl.c
>>>         +++ b/modules/hw/nvdec/nvdec_gl.c
>>>         @@ -150,7 +150,8 @@ static void Close(vlc_object_t *obj)
>>>         {
>>>         struct vlc_gl_interop *interop = (void *)obj;
>>>         converter_sys_t *p_sys = interop->priv;
>>>         - vlc_decoder_device_Release(p_sys->device);
>>>         + if (p_sys)
>>>         + vlc_decoder_device_Release(p_sys->device);
>>>
>>>
>>>     This is very suspicious. p_sys is owned by this object and not 
>>> used by
>>>     anyone else. It's allocated with vlc_obj_malloc() so doesn't 
>>> need to be
>>>     freed manually and is valid as long the object is valid.
>>>
>>>     If the pointer is wonrg it's a sign something is not working right
>>>     elsewhere.
>>>
>>>         }
>>>
>>>         static int Open(vlc_object_t *obj)
>>>         --         2.17.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
>>>
>>>
>>> _______________________________________________
>>> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel