[FFmpeg-devel] [PATCH 0/3] Finish new iteration APIs

[Michael Niedermayer]

On Wed, Feb 21, 2018 at 09:27:02AM +0100, wm4 wrote:
> On Tue, 20 Feb 2018 21:45:12 +0100
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Tue, Feb 20, 2018 at 06:28:20PM +0100, wm4 wrote:
> > > On Tue, 20 Feb 2018 17:30:32 +0100
> > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > >   
> > > > On Tue, Feb 20, 2018 at 10:17:02AM -0300, James Almer wrote:  
> > > > > On 2/20/2018 9:21 AM, wm4 wrote:    
> > > > > > On Tue, 20 Feb 2018 08:47:51 -0300
> > > > > > James Almer <jamrial at gmail.com> wrote:
[...]
> > > And for the 100th time: the new API is completely orthogonal to
> > > allowing user-registered components. Since nobody could actually use
> > > the API before, it's no problem to drop the old APIs now, and to add
> > > actually working API once the other, much much much bigger problems are
> > > solved.
> > > 
> > > Even if you argue that keeping the linked list is absolutely necessary,
> > > the new API could support a linked list too.
> > >   
> > > > is it the ff_* symbols you are thinking of ?  
> > > 
> > > ff_ symbols are private.
> > >   
> > > > This is a problem for an existing API it is not a problem for a new API as
> > > > we can change the symbols if they are intended to be used for individual
> > > > component registration.
> > > > The whole discussion is about designing a new API. So any limitation of
> > > > an existing API can be changed.
> > > > 
> > > > There also should be no need to call register_all in the existing API,
> > > > and there cannot be such a problem in a new design because it would be
> > > > a new design you wouldnt design it to "not work".  
> > > 
> > > You're just being contradictory all across the board here. In other
> > > places you're claiming this register mechanism is needed for
> > > security or to make it possible to eliminate unused components when
> > > static linking. But if all components are already registered without
> > > doing anything, how is that supposed to work?  
> > 
> > If an application wants to register all, it calls the register_all()
> > function
> > If an application wants to register a subset it registers just that
> > subset and does not call register_all
> 
> But you just said on your mail "There also should be no need to call
> register_all in the existing API,". How is that supposed to mesh with 
> "If an application wants to register a subset it registers just that
> subset and does not call register_all".

I see no contradiction between what you quote, if that is what you meant:
"There also should be no need to call register_all in the existing API,"
and
"If an application wants to register a subset it registers just that subset and does not call register_all"


> 
> But indeed one goal was that applications don't have to call the
> completely pointless register_all functions.

> 
> > And even the documentation says this currently:
> > /**
> >  * Register all the codecs, parsers and bitstream filters which were enabled at
> >  * configuration time. If you do not call this function you can select exactly
> >  * which formats you want to support, by using the individual registration
> >  * functions.
> >  *
> 
> Explain how that could actually work with dynamic linking.

If you see an issue with dynamic linking, please
explain what you mean, and i will try to show that it does not
affect a newly designed API.
Fundamentally, one can of course proof this simply by showing that
other libs do have working registration style functions even when
dynamically linked.


> 
> > 
> > >   
> > > >   
> > > > > 
> > > > > Nevermind then, my argument was focused on preventing losing some link
> > > > > time optimization for static libraries assuming proper API usage.
> > > > >     
> > > > > > 
> > > > > > And that can't be made with dynamic linking either. If you statically
> > > > > > link anyway, you probably control everything, and you can configure this
> > > > > > stuff at compile time.
> > > > > > 
> > > > > > Then I guess there's this very special case where a fuzzer statically
> > > > > > links to libavcodec, and registers only a few components (technically
> > > > > > violating the API and by guessing the component symbol name), and
> > > > > > without calling the register_all functions. This would make the
> > > > > > resulting executable much smaller, which is apparently important
> > > > > > because Google (who runs the fuzzers for their oss-fuzz project) is too
> > > > > > poor (?) to host all the statically linked fuzzers, _or_ the whitelist
> > > > > > stuff is not enough to trick the fuzzer into not trying to fuzz the
> > > > > > wrong code. In addition, it's apparently infeasible to just build
> > > > > > every fuzzer with a separate libavcodec. (Not sure about the details, it
> > > > > > was something like this.)
> > > > > > 
> > > > > > Those requirements are really quite nebulous. I don't know why we even
> > > > > > should care - surely whoever does this will find a solution that works
> > > > > > for them. For example they could apply a small patch that makes the
> > > > > > codec_list[] symbol non-static non-const, which would allow them to
> > > > > > overwrite it (i.e. deleting unneeded entries). It's a really simple
> > > > > > solution that took me 5 minutes to come up with.    
> > > > > 
> > > > > Something like this is probably the best solution for the fuzzer issue, yes.    
> > > > 
> > > > This basically says one should fork ffmpeg if one has problems with the new API  
> > > 
> > > What? Jesus christ, stop being so obstinate and playing dumb just to
> > > try to prove your non-existent point.
> > > 
> > > Applying a patch for a very special setup is not a problem.   
> > 
> > 
> > > Most of the
> > > serious projects using FFmpeg apply custom patches anyway.  
> > 
> > And this should maybe give us a hint that we have a problem that we
> > should solve.
> 
> No, because they have special requirements and do things that wouldn't
> make sense to do upstream, or want to do things that would not be
> reasonable to do upstream.

This is quite off topic but what you say doesnt fit together

* you said 
  "Most of the serious projects using FFmpeg apply custom patches anyway."
  and then various justifications ...
but distributions ship a single shared set a FFmpeg libraries in general
they dont allow or want many copies of patched libs.

So that would mean major distributions do not ship what you called 
"serious projects using FFmpeg", i think that would be absurd and cannot be
true. 

And more so, there are very good reasons why distributions do not want
multiple patched variants of libs, its a nightmare to track this and do
security updates. Ever saw debian update 50 packages when a security update
for libavcodec was released ? NO because they dont really allow this

And this is also why i said 
"And this should maybe give us a hint that we have a problem that we should solve."
That is if there are applications that patch our libs, they will loose these
patches in distributions like debian or they will loose security support from
distributions. This is not good and it is something we should solve if there are such
cases.



[...]

> > I tend to avoid arguing over your statments as it has in the past
> > led to an escalation of attacks, insults and that helps noone.
> > But it seems you are upset when i dont reply, and you are upset if i
> > do reply.
> 
> Littering me with obstinate non-sequitur replies and inconsistent
> arguments, and then insinuating that I got angry, and using that to
> claim that I'm acting unreasonable is not a nice way to discuss at all.
> Are you sure you're even interested in factual discussion?

Iam interrested in factual discussion. But as we can see here it does not
work that well. And so i avoid a bit to reply to you. This doesnt mean i
dont read your suggestions or ignore them just that i dont want to get
pulled into these kind of threads.


> 
> For example take this case: you claimed something would stop working, I
> supplied a simple solution for it, and then you ignored that I did and
> continued to claim that this case was broken for you.

And there are so many inaccurate things in here too.
I claimed that the commited patchset significantly increase the size of
the "fuzzer tools" and that the diskspace there is limited.

1. You turn this into a "you claimed something would stop working", i
  did not say that though it very possibly could stop working or it
  could restrict future extensions like testing encoders or parsers.
  
2. "I supplied a simple solution for it, and then you ignored that"
    IMO, a solution is a change to the code, you did not supply that.
    you supplied some idea(s), there already are multiple ideas on
    how to workarouind or solve it.

3. "and continued to claim that this case was broken for you."
  Again i said the files are many times bigger and they where at the
  time i said that. Thats solved when its solved not when someone
  publishes an idea on how it could be solved.

Now how is this sort of arguing relevant to a "factual discussion"
most of your example here is not factual

More so the factual discussion in this thread should be about designing
a better API. Large parts of this are not contributing to that

    
> 
> Sorry if I'm calling you out for it. I don't think that necessarily
> constitutes and "attack" or an "insult", it just seems to serve you to
> actually attack and insult me.
> 
> And I'm tired of that.
> 
> > > 
> > > Regarding security assuming a dynamically linked distro FFmpeg build:
> > > 
> > > Registering wouldn't help you, because any other library in the same
> > > process could just register all components.  
> > 
> > yes but there is no such library in almost all these cases.
> 
> Sure, if you can guarantee that it's this way - otherwise it won't
> work. But it's actually pretty common in multimedia settings to load
> other things as plugins. Consider MediaFoundation/DirectShow (which have
> FFmpeg plugins, and many other plugins), gstreamer, or VLC.
> 
> So I would say your "almost all" is not correct.

security concious applications would likely run FFmpeg and its libs in
a seperate process. So there is not going to be other things in it.

And security concious applications which do not will very likely be
very restrictiv on what they link to. Its unlikely they will
pull in huge unrelated plugin systems as that runs against the idea of
security.

But as you say "Sure, if you can guarantee that it's this way"
In fact its easy to gurantee this so as to eliminate all of these
corner cases.

you only need a register_lock() which disables registering any further
components.
Then the application just registers what it needs and calls
if (register_lock() != "number of codecs the app registered")
    fatal();
    
Now if a lib attempts to register a component after this it would
only succeed if that component was already registered and fail otherwise.
So no lib can inject more components than what the application authorized


> 
> > 
> > > If you relied on
> > > conditional registration, your security could be randomly broken and
> > > you wouldn't even notice this. Consider interesting cases like ALSA
> > > loading a plugin that uses libavcodec, or so.  
> > 
> > Its an additional layer, not something to rely on alone
> 
> OK, so you basically admit that it doesn't work.

no, thats your interpretation. Maybe what you want to hear, i dont know but
its not what i said.
and this kind of reinterpreting things i say, is not a basis for discussion.

Thanks

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

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180221/64727eff/attachment.sig>