On Tue, Sep 30, 2014 at 06:23:38PM +0200, wm4 wrote:
> On Tue, 30 Sep 2014 17:21:20 +0200
> Michael Niedermayer <michae...@gmx.at> wrote:
>
> > Hi
> >
> > On Tue, Sep 30, 2014 at 03:51:13PM +0200, wm4 wrote:
> > > On Tue, 30 Sep 2014 02:02:49 +0200
> > > Michael Niedermayer <michae...@gmx.at> wrote:
> > >
> > > > Signed-off-by: Michael Niedermayer <michae...@gmx.at>
> > > > ---
> > > > libavcodec/allcodecs.c | 51
> > > > +++++++++++++++++++++++++++++++++---------------
> > > > libavcodec/avcodec.h | 5 +++++
> > > > 2 files changed, 40 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > > > index 7650543..019d5ea 100644
> > > > --- a/libavcodec/allcodecs.c
> > > > +++ b/libavcodec/allcodecs.c
> > > > @@ -29,49 +29,55 @@
> > > > #include "version.h"
> > > >
> > > > #define REGISTER_HWACCEL(X, x)
> > > > \
> > > > - {
> > > > \
> > > > + if (!name || !strcmp(name, AV_STRINGIFY(x##_hwaccel))) {
> > > > \
> > > > extern AVHWAccel ff_##x##_hwaccel;
> > > > \
> > > > - if (CONFIG_##X##_HWACCEL)
> > > > \
> > > > + if (CONFIG_##X##_HWACCEL) {
> > > > \
> > > > av_register_hwaccel(&ff_##x##_hwaccel);
> > > > \
> > > > + found++;
> > > > \
> > > > + }
> > > > \
> > > > }
> > > >
> > > > #define REGISTER_ENCODER(X, x)
> > > > \
> > > > - {
> > > > \
> > > > + if (!name || !strcmp(name, AV_STRINGIFY(x##_encoder))) {
> > > > \
> > > > extern AVCodec ff_##x##_encoder;
> > > > \
> > > > - if (CONFIG_##X##_ENCODER)
> > > > \
> > > > + if (CONFIG_##X##_ENCODER) {
> > > > \
> > > > avcodec_register(&ff_##x##_encoder);
> > > > \
> > > > + found++;
> > > > \
> > > > + }
> > > > \
> > > > }
> > > >
> > > > #define REGISTER_DECODER(X, x)
> > > > \
> > > > - {
> > > > \
> > > > + if (!name || !strcmp(name, AV_STRINGIFY(x##_decoder))) {
> > > > \
> > > > extern AVCodec ff_##x##_decoder;
> > > > \
> > > > - if (CONFIG_##X##_DECODER)
> > > > \
> > > > + if (CONFIG_##X##_DECODER) {
> > > > \
> > > > avcodec_register(&ff_##x##_decoder);
> > > > \
> > > > + found++;
> > > > \
> > > > + }
> > > > \
> > > > }
> > > >
> > > > #define REGISTER_ENCDEC(X, x) REGISTER_ENCODER(X, x);
> > > > REGISTER_DECODER(X, x)
> > > >
> > > > #define REGISTER_PARSER(X, x)
> > > > \
> > > > - {
> > > > \
> > > > + if (!name || !strcmp(name, AV_STRINGIFY(x##_parser))) {
> > > > \
> > > > extern AVCodecParser ff_##x##_parser;
> > > > \
> > > > - if (CONFIG_##X##_PARSER)
> > > > \
> > > > + if (CONFIG_##X##_PARSER) {
> > > > \
> > > > av_register_codec_parser(&ff_##x##_parser);
> > > > \
> > > > + found++;
> > > > \
> > > > + }
> > > > \
> > > > }
> > > >
> > > > #define REGISTER_BSF(X, x)
> > > > \
> > > > - {
> > > > \
> > > > + if (!name || !strcmp(name, AV_STRINGIFY(x##_bsf))) {
> > > > \
> > > > extern AVBitStreamFilter ff_##x##_bsf;
> > > > \
> > > > - if (CONFIG_##X##_BSF)
> > > > \
> > > > + if (CONFIG_##X##_BSF) {
> > > > \
> > > > av_register_bitstream_filter(&ff_##x##_bsf);
> > > > \
> > > > + found++;
> > > > \
> > > > + }
> > > > \
> > > > }
> > > >
> > > > -void avcodec_register_all(void)
> > > > +int avcodec_register_one(const char *name)
> > > > {
> > > > - static int initialized;
> > > > -
> > > > - if (initialized)
> > > > - return;
> > > > - initialized = 1;
> > > > + int found = 0;
> > > >
> > > > /* hardware accelerators */
> > > > REGISTER_HWACCEL(H263_VAAPI, h263_vaapi);
> > > > @@ -588,4 +594,17 @@ void avcodec_register_all(void)
> > > > REGISTER_BSF(NOISE, noise);
> > > > REGISTER_BSF(REMOVE_EXTRADATA, remove_extradata);
> > > > REGISTER_BSF(TEXT2MOVSUB, text2movsub);
> > > > +
> > > > + return found;
> > > > }
> > > > +
> > > > +void avcodec_register_all(void)
> > > > +{
> > > > + static int initialized;
> > > > +
> > > > + if (initialized)
> > > > + return;
> > > > + initialized = 1;
> > > > +
> > > > + avcodec_register_one(NULL);
> > > > +}
> > > > \ No newline at end of file
> > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > index 94e82f7..de936a5 100644
> > > > --- a/libavcodec/avcodec.h
> > > > +++ b/libavcodec/avcodec.h
> > > > @@ -3472,6 +3472,11 @@ void avcodec_register(AVCodec *codec);
> > > > void avcodec_register_all(void);
> > > >
> > > > /**
> > > > + * @returns the number of newly registered entities
> > > > + */
> > > > +int avcodec_register_one(const char *name);
> > > > +
> > > > +/**
> > > > * Allocate an AVCodecContext and set its fields to default values. The
> > > > * resulting struct should be freed with avcodec_free_context().
> > > > *
> > >
> > > What's the purpose? It requires global mutable state, and won't work is
> > > more than one library in a process is using ffmpeg. So this would just
> > > add a fundamentally misdesigned API.
> > >
> >
> > > If the purpose is to restrict the codecs that can be used (maybe for
> > > security reasons?),
> >
> > yes, its intended for security purposes
> >
> >
> > > then it should be done in a safe manner
> >
> > yes
> >
> >
> > > with no
> > > global mutable state.
> >
> > i disagree, applications that care about security must
> > restrict used entities globally, thus by definition want to change
> > global state. maybe not the way its done in this patch yes, ill
> > think about it and submit something better but
>
> No, it doesn't work.> Code using libavcodec can and will call > avcodec_register_all(). yes, but avcodec_register_all() would then just not register anything that has been restricted before. There never was a gurantee that it would register some specific decoder as it could have been disabled at compile time > > > A security critical application does not want a mysterious unknown > > library to load and use a unrestricted libavcodec behind its back > > that would likely bypass the whole idea of restricting the decoders > > and demuxers > > The only reasonable way to do that is to make sure the whole thing is > secure at build time. That is, strip everything unneeded or unwanted > from ffmpeg when configuring, and making sure everything in the program > is using this ffmpeg binary. thats disallowed per policy in some distributions like debian AFAIK libs must be build as shared libs and shared, no local copies allowed if avoidable or something unless i misremember also there are reasons for this, doig security maintaince for all these copies would be a nightmare for a distribution > > You can not decide this at runtime and with an unknown set of > libraries. Unless the library you're using is designed to enable this > use case explicitly and won't call avcodec_register_all() on its own. > But then you might as well create a better API, like a per > AV(Codec/Format)Context whitelist of allowed codecs. > > > > > > I can't see any other purpose; it won't prevent > > > "uneeded" codecs from being pulled in by the linker. > > > > > > > > Even worse, it would make it impossible to fix the thread-safety and > > > library-safety issues of avcodec_register_all(). > > > > what issues are you talking about? please elaborate > > What happens if two libraries call avcodec_register_all() at the same > time in separate threads? The individual register functions are thread safe, the register_all isnt, which is not good i agree > > Oh well, you could probably add a hack to mitigate this, but the point > is that this is unnecessary global state and could be avoided entirely. > There's literally no reason for it to exist. yes anyway ive posted another patchset before reading this but i think it should resolve some of the points raised here > > (A mitigation hack wouldn't fix this completely, because compiling > without threading APIs is still supposed to result in thread-safe > library, at least if this awful libavcodec "lock manager" thing is > used.) > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
