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

[Michael Niedermayer]

On Sun, Nov 09, 2008 at 03:44:43PM +0100, Stefano Sabatini wrote:
> On date Sunday 2008-11-09 13:36:39 +0100, Michael Niedermayer encoded:
> > On Sun, Nov 09, 2008 at 12:48:37PM +0100, Stefano Sabatini wrote:
> > > On date Thursday 2008-10-30 23:07:29 +0100, Michael Niedermayer encoded:
> > > > On Thu, Oct 30, 2008 at 09:05:32PM +0100, Stefano Sabatini wrote:
> > > [...]
> > > > > 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.
> > > 
> > > I'll try to explain myself better...
> > > 
> > > av*_opts and swscale_opts are declared and defined (as NULL) in
> > > cmdutils.c, applications linking to it have to define them to
> > > something different from NULL or they will crash when calling
> > > opt_default().
> > > 
> > > Patching libavfilter-soc would mean in this case something like the
> > > following in cmdutils.c:
> > > 
> > > #ifndef CONFIG_AVFILTER
> > >     if(!o)
> > >         o = av_set_string2(sws_opts, opt, arg, 1);
> > > #endif
> > > 
> > > which I think it's just plain wrong, since cmdutils.o should be
> > > application agnostic, and there is no reason for which we shouldn't
> > > look in sws_opts simply because CONFIG_AVFILTER is set.
> > 
> > there is NO sws_opts when avfilter is in use! The variable does NOT exist
> > semantically.
> 
> Currently sws_opts is declared as extern in cmdutils.h and defined in
> cmdutils.c, then used in the ff*.c tools, so the variable *does* exist
> semantically for every application, with or without

it does not exist _semantically_ in the sense that 0 has no logarithm.


[...]
> I'm not sure, but I remember vaguely that in some older
> libavfilter-soc version sws_opts declaration and use was under an #if
> !ENABLE_AVFILTER.
> 
> After your patch which moved opt_default() and *_opts to
> cmdutils.[ch], in oder to get the same effect we should patch
> cmdutils.[ch], which as I already wrote I don't think is a good idea.

when libavfilters scale filter will be included in svn then ALL WITHOUT
EXCEPTION references to sws_opts will be removed at the same time.
before this happens sws_opts will be initialized!


> 
> > Thats simply because the corresponding command line option does NOT exist
> > when avfilter is enabled.
> 
> Which commandline option? Do you mean sws_flags?

yes

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- 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/20081109/924633b1/attachment.pgp>