[vlc-devel] [PATCH] input: Fix race condition on input item titles

[Rémi Denis-Courmont]

Le sunnuntaina 27. elokuuta 2017, 11.02.50 EEST Rémi Denis-Courmont a écrit :
> Le perjantaina 25. elokuuta 2017, 15.42.59 EEST Hugo Beauzée-Luyssen a écrit 
:
> > Fix #18727
> > ---
> > 
> >  src/input/input.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/input/input.c b/src/input/input.c
> > index ae5fe03f81..af416b4b54 100644
> > --- a/src/input/input.c
> > +++ b/src/input/input.c
> > @@ -2269,7 +2269,10 @@ static void UpdateGenericFromDemux( input_thread_t
> > *p_input )
> > 
> >  static void UpdateTitleListfromDemux( input_thread_t *p_input )
> >  {
> > 
> > -    input_source_t *in = input_priv(p_input)->master;
> > +    input_thread_private_t *priv = input_priv(p_input);
> > +    input_source_t *in = priv->master;
> > +
> > +    vlc_mutex_lock( &priv->p_item->lock );
> > 
> >      /* Delete the preexisting titles */
> >      if( in->i_title > 0 )
> > 
> > @@ -2288,6 +2291,7 @@ static void UpdateTitleListfromDemux( input_thread_t
> > *p_input ) else
> > 
> >          in->b_title_demux = true;
> > 
> > +    vlc_mutex_unlock( &priv->p_item->lock );
> > 
> >      InitTitle( p_input );
> >  
> >  }
> 
> I don´t see where the item is used, so then there is no point taking its
> lock.

There is in fact a not very good design decision to use the master input item 
lock to protect some properties of the input thread. But that does not seem to 
be the case for ->titles.

Also, the lock cannot be taken while calling the demuxer: The demuxer is 
allowed to acquire that lock - potentially leading to undefined recursive 
locking or lock inversion.

-- 
雷米‧德尼-库尔蒙
https://www.remlab.net/