[vlc-devel] [PATCH] lib: media_player: retain libvlc instance when switching

Alexandre Janniaux ajanni at videolabs.io
Thu Jul 9 09:51:33 CEST 2020


Hi,

Indeed, you're right, it worked from my side when testing
because I still have the libvlc instance, so it mostly avoid
the double free/use-after-free situation I describe but it
doesn't solve the VLCKit case as the player has a libvlc
instance which is not refcounted outside of it -- which
is the use-after-free you describe.

I wanted to solve the crash before having to rework this
whole part since the code is old and information are scarce
around this, but I guess I have no choice here.

Thank you for your review!

Regards,
--
Alexandre Janniaux
Videolabs

On Wed, Jul 08, 2020 at 06:27:38PM +0200, Rémi Denis-Courmont wrote:
> Hi,
>
> Not sure what the code tries to do here, but you can't switch the instance underneath a player since the involved underlying VLC objects are tied to the original instance.
>
> This patch seems as wrong as the current code.
>
> Le 8 juillet 2020 17:24:44 GMT+02:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
> >The media_player instance retains and releases the libvlc instance it's
> >created from. When using a different libvlc instance for media_player
> >and media, it's using the libvlc instance from the media_t object,
> >leading to the release of the wrong libvlc instance, and thus potential
> >use-after-free of one instance and leaks of the other.
> >
> >It has been spotted since VLCKit creates a shared libvlc instance and
> >then can create a new libvlc instance in case the VLCMediaPlayer is
> >created with different options, which means that the VLCMediaPlayer and
> >the VLCMedia will be bound to different libvlc instances, triggering
> >the
> >issue described in first paragraph and crashing.
> >
> >Refs videolan/VLCKit#189, videolan/VLCKit#116
> >---
> > lib/media_player.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> >diff --git a/lib/media_player.c b/lib/media_player.c
> >index 9c7a7a76aad..3e8636facea 100644
> >--- a/lib/media_player.c
> >+++ b/lib/media_player.c
> >@@ -914,6 +914,8 @@ void libvlc_media_player_set_media(
> >
> > /* The policy here is to ignore that we were created using a different
> >      * libvlc_instance, because we don't really care */
> >+    libvlc_retain(p_md->p_libvlc_instance);
> >+    libvlc_release(p_mi->p_libvlc_instance);
> >     p_mi->p_libvlc_instance = p_md->p_libvlc_instance;
> >
> >     vlc_player_Unlock(p_mi->player);
> >--
> >2.27.0
> >
> >_______________________________________________
> >vlc-devel mailing list
> >To unsubscribe or modify your subscription options:
> >https://mailman.videolan.org/listinfo/vlc-devel
>
> --
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list