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

[Alexandre Janniaux]

Actually, the instance is still alive and destroyed after
the player so it fixes VLCKit, but it's still broken with
regards to general libvlc usage.

Regards,
--
Alexandre Janniaux
Videolabs

On Thu, Jul 09, 2020 at 09:51:33AM +0200, Alexandre Janniaux wrote:
> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel