[vlc-devel] [PATCH 2/2] sout: description: make the ownership of es_format clear

[Zhao Zhili]

> On Oct 1, 2018, at 6:20 PM, Francois Cartegnie <fcvlcdev at free.fr> wrote:
> 
> Le 30/09/2018 à 07:01, Zhao Zhili a écrit :
> 
>> 
>> -    TAB_APPEND( p_sys->data->i_es, p_sys->data->es, p_fmt_copy );
>> +    es_format_t *p_fmt_copy2 = malloc( sizeof( *p_fmt_copy2 ) );
>> +    if( unlikely(p_fmt_copy2 == NULL ) )
>> +        goto error;
>> +
>> +    if( es_format_Copy( p_fmt_copy2, p_fmt ) != VLC_SUCCESS )
>> +    {
>> +        free( p_fmt_copy2 );
>> +        goto error;
>> +    }
>> +    TAB_APPEND( p_sys->data->i_es, p_sys->data->es, p_fmt_copy2 );
>> 
>>     if( p_sys->i_stream_start == VLC_TICK_INVALID )
>>         p_sys->i_stream_start = vlc_tick_now();
>> 
>>     return (void *)p_fmt_copy;
>> +
>> +error:
>> +    es_format_Clean( p_fmt_copy );
>> +    free( p_fmt_copy );
>> +    return NULL;
>> }
>> 
>> static void Del( sout_stream_t *p_stream, void *id )
>> {
>>     msg_Dbg( p_stream, "Removing a stream" );
>> -    /* NOTE: id should be freed by the input manager, not here. */
>> -    (void) id;
>> +    es_format_Clean( (es_format_t *)id);
>> +    free( id );
>> }
>> 
>> static int Send( sout_stream_t *p_stream, void *id, block_t *p_buffer )
>> 
> 
> To me looks incorrect.
> You want to keep track of created void *id locally, you don't need 2 copies.
> 
> Only the NOTE is incorrect and the Del() must free the opaque, being
> here a fmt copy.
> 

Del() is called during input_Close(), but those es_format associated with sout-description-data are used after input_Close(). So if there is a single copy and Del() free those es_format, it leads to use-after-free.
If you think two copies are not right, any other suggestion?

vlm.c:
                var_Create( p_input, "sout-description-data", VLC_VAR_ADDRESS );
                var_SetAddress( p_input, "sout-description-data", &data );

                if( !input_Start( p_input ) )
                    vlc_sem_wait( &sem_preparse );

                var_DelCallback( p_input, "intf-event", InputEventPreparse,
                                 &preparse );

                input_Stop( p_input );
                input_Close( p_input );


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