[vlc-devel] [PATCH] decoder: fix NULL dereference when video format is updating

[Rémi Denis-Courmont]

Le lundi 27 avril 2015, 19:14:44 Thomas Guillem a écrit :
> On Mon, Apr 27, 2015, at 18:25, Rémi Denis-Courmont wrote:
> > Le lundi 27 avril 2015, 18:08:44 Thomas Guillem a écrit :
> > > p_owner->p_vout can be set to NULL by decoder_UpdateVideoFormat that can
> > > be
> > > called from a different thread than the decoder one (see avcodec).
> > 
> > If the current reads p_owner->p_vout (at line 861 and also in the calling
> > function), then p_owner->p_vout must be read-only.
> > 
> > So this is a bug in avcodec, or a limitation of the decoder core
> > depending on
> > your point of view. Either way, the patch does not seem "sufficient" to
> > me.
> 
> OK, it's not sufficient because p_vout should be written only by the
> DecoderThread (for now).

I guess it can be written by another thread when:
- the decoder thread is inside pf_decode_video(), thus ensuring that the 
decoder thread will not access the vout, and
- the owner lock is held, thus ensuring that the decoder owner (the input 
thread or whatever) will not access the vout, nor will any other threaded 
decoder plugin thread (if it exists).

But at any other times, the decoder thread can indeed read the value without 
lock, so vout pointer must be read-only.

> If it's not the case anymore, we should add
> locks everytime we read p_vout from this thread. Am I right ?

Yes.

And also somehow ensure that pictures from a previous vout (before last format 
update) is not queued to the current vout. It could be argued that threaded 
decoder plugins should be responsible for this though.

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