[vlc-devel] [PATCH v3 2/2] access/http: Share cookies between all playlist items

[Rémi Denis-Courmont]

	Moikka,

Le dimanche 14 septembre 2014, 10:27:42 Antti Ajanki a écrit :
> diff --git a/src/playlist/engine.c b/src/playlist/engine.c
> index de1f76b..8d6928c 100644
> --- a/src/playlist/engine.c
> +++ b/src/playlist/engine.c
> @@ -32,6 +32,7 @@
>  #include <vlc_sout.h>
>  #include <vlc_playlist.h>
>  #include <vlc_interface.h>
> +#include <vlc_http.h>
>  #include "playlist_internal.h"
>  #include "input/resource.h"
> 
> @@ -296,6 +297,15 @@ playlist_t *playlist_Create( vlc_object_t *p_parent )
>      if( aout != NULL )
>          input_resource_PutAout( p->p_input_resource, aout );
> 
> +    /* Initialize the shared HTTP cookie jar */
> +    vlc_value_t cookies;
> +    cookies.p_address = vlc_http_cookies_new();
> +    if ( likely(cookies.p_address) )
> +    {
> +        var_Create( p_playlist, "http-cookies", VLC_VAR_ADDRESS );
> +        var_SetChecked( p_playlist, "http-cookies", VLC_VAR_ADDRESS,
> cookies ); +    }
> +
>      /* Thread */
>      playlist_Activate (p_playlist);
> 
> @@ -366,6 +376,13 @@ void playlist_Destroy( playlist_t *p_playlist )
>      ARRAY_RESET( p_playlist->items );
>      ARRAY_RESET( p_playlist->current );
> 
> +    vlc_http_cookie_jar_t *cookies = var_GetAddress( p_playlist,
> "http-cookies" );
> +    if ( cookies )
> +    {
> +        var_Destroy( p_playlist, "http-cookies" );
> +        vlc_http_cookies_release( cookies );

The reference counting does not work here. Considering the variable alone, you 
could have:

B calls var_InheritAddress("http-cookies"), gets a non-NULL pointer
A calls var_GetAddress("http-cookies")
A calls var_Destroy("http-cookies")
A calls vlc_http_cookies_release()
-> cookies jar deletion
B calls vlc_http_cookies_retain()
-> use-after-free and probably abort inside vlc_mutex_lock().

So either there is a warranty that when playlist_Destroy() is called, no 
thread can be fetching cookies any longer. In that case, the reference counter 
is pointless. Or there is no such warranty and the code is just wrong.

Hopefully, the earlier. Then don't bother with the reference count.

-- 
Rémi Denis-Courmont
http://www.remlab.net/