[vlc-devel] [PATCH 1/7] objects: add back legacy list node

[Rémi Denis-Courmont]

Le maanantaina 27. toukokuuta 2019, 18.04.32 EEST Thomas Guillem a écrit :
> On Mon, May 27, 2019, at 17:00, Rémi Denis-Courmont wrote:
> > Le maanantaina 27. toukokuuta 2019, 17.45.10 EEST Thomas Guillem a écrit :
> > > This has been removed from c7e98fa75a88466b94b1bafae3bac3ba5f634537 as a
> > > side effect.
> > 
> > The object tree served exaxtly one purpose: to find other objects. We
> > don't do that anymore because it's intrinsically racy and led to
> > unfixable bugs in RC and LibVLC filters.
> > 
> > And this has nothing to do with finding the name of a module, which you
> > can do with module_get_name() as always.
> > 
> > > @@ -196,11 +212,13 @@ size_t vlc_list_children(vlc_object_t *obj,
> > > vlc_object_t **restrict tab, vlc_object_internals_t *priv;
> > > 
> > >      size_t count = 0;
> > > 
> > > +    vlc_mutex_lock(&tree_lock);
> > > 
> > >      vlc_children_foreach(priv, vlc_internals(obj))
> > >      {
> > >      
> > >           if (count < max)
> > >           
> > >               tab[count] = vlc_object_hold(vlc_externals(priv));
> > >           
> > >           count++;
> > >      
> > >      }
> > > 
> > > +    vlc_mutex_unlock(&tree_lock);
> > > 
> > >      return count;
> > 
> > Use after free here.
> 
> It's ok since this function is removed just after.
> 
> I can modify this patch to not touch vlc_list_children.
> 
> > Nack.
> 
> I'm not sure you can decide that alone.

That's moot because I did not decide that alone.

Besides it's a fact, not a decision, that it's racy and that the later snippet 
leads to use after free.

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