[FFmpeg-devel] [PATCH] Fix potential av_find_opt() crash if context is NULL

[Michael Niedermayer]

On Thu, Oct 30, 2008 at 09:05:32PM +0100, Stefano Sabatini wrote:
> On date Thursday 2008-10-30 19:54:56 +0100, Michael Niedermayer encoded:
> > On Thu, Oct 30, 2008 at 04:33:05PM +0100, Stefano Sabatini wrote:
> > > On date Thursday 2008-10-30 14:32:29 +0100, Michael Niedermayer encoded:
> > > > On Thu, Oct 30, 2008 at 12:14:36AM +0100, Stefano Sabatini wrote:
> > > > > On date Wednesday 2008-10-29 00:43:16 +0100, Michael Niedermayer encoded:
> > > > > > On Wed, Oct 29, 2008 at 12:27:35AM +0100, Stefano Sabatini wrote:
> > > > > > > Hi, as in patch.
> > > > > > > 
> > > > > > > This happens for example in libavfilter-soc with:
> > > > > > > ffplay -foo x
> > > > > > > 
> > > > > > > when --enable-avfilter, in this case sws_opts is NULL.
> > > > > > 
> > > > > > could you elaborate a little more how above is causing a NULL
> > > > > > to be passed.
> > > > > > All readers surely could guess but i think it would improve the
> > > > > > responses if everyone would know what happens where 
> > > > > 
> > > > > We have:
> > > > > 
> > > > > #if !ENABLE_AVFILTER
> > > > >     sws_opts = sws_getContext(16,16,0, 16,16,0, sws_flags, NULL,NULL,NULL);
> > > > > #endif
> > > > > 
> > > > > So sws_opts is not initialized and is NULL.
> > > > > 
> > > > > Then when an unknown option is parsed and processed in opt_default we
> > > > > have:
> > > > > 
> > > > >     if(!o)
> > > > >         o = av_set_string2(sws_opts, opt, arg, 1);
> > > > > 
> > > > > which calls av_find_opt() and crashes.
> > > > > 
> > > > > Optionally we could specify in the doc the condition for which
> > > > > opt_find_opt has to take always a non-NULL value as obj, but I think
> > > > > it is more robust and semnatically more correct to simply return NULL
> > > > > in case of a NULL obj.
> > > > 
> > > > bleh ...
> > > > the problem is obvious in the ENABLE_AVFILTER #ifdefs
> > > > if sws is not used by ffmpegm sws_opts itself should not even exist with
> > > > avfilter
> > > 
> > > Mhh.. so what about this? And I'm not sure it is the best solution...
> > 
> > i think the non null-ness is already implied by the first element being
> > something.
> 
> OK, but then we should check for its nullness every time we access to
> it in cmdline.c, since that's generic code and from there we can't
> know if sws_opts has been initialized or not, same consideration for
> avformat_opts and avcodec_opts.
> 
> *Or*, we can change the semantics of av_find_opt() (micro bump) to
> make it accept a NULL, for which solution I have a slight preference.

i disagree, its all fine as it is, NULL is not allowed and that should be
clear from the docs,
the avfilter patch is broken its the onle thing that need fixing.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081030/8efe8b06/attachment.pgp>