diff mbox

[FFmpeg-devel,PATCHv2] avfilter/formats: allow unknown channel layouts by default

Message ID 20161208010719.6485-1-cus@passwd.hu
State Accepted
Commit 7ceb9e6b11824ff18f424a35e41fbddf545d1238
Headers show

Commit Message

Marton Balint Dec. 8, 2016, 1:07 a.m. UTC 
Since the default in the libav fork is to only allow known layouts, making
unknown layouts allowed by default here can be a security risk for filters
directly merged from libav. However, usually it is simple to detect such cases,
use of av_get_channel_layout_nb_channels is a good indicator, so I suggest we
change this regardless.

See http://ffmpeg.org/pipermail/ffmpeg-devel/2016-November/203204.html.

This patch indirectly adds unknown channel layout support for filters where
query_formats is not specified:

abench
afifo
ainterleave
anullsink
apad
aperms
arealtime
aselect
asendcmd
asetnsamples
asetpts
asettb
ashowinfo
azmq

It introduces a query_formats callback for the asyncts filter, which only
supports known channel layouts since it is using libavresample.

And it removes .query_formats callback from filters where it was only there to
support unknown layouts, as this is now the default:

aloop
ametadata
anull
asidedata
asplit
atrim

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/af_anull.c   |  1 -
 libavfilter/af_asyncts.c |  1 +
 libavfilter/f_loop.c     |  1 -
 libavfilter/f_metadata.c |  1 -
 libavfilter/f_sidedata.c |  1 -
 libavfilter/formats.c    |  6 +++---
 libavfilter/formats.h    | 12 +++++-------
 libavfilter/split.c      |  1 -
 libavfilter/trim.c       |  1 -
 9 files changed, 9 insertions(+), 16 deletions(-)

Comments

Nicolas George Dec. 9, 2016, 10:37 a.m. UTC | #1 
L'octidi 18 frimaire, an CCXXV, Marton Balint a écrit :
> Since the default in the libav fork is to only allow known layouts, making
> unknown layouts allowed by default here can be a security risk for filters
> directly merged from libav. However, usually it is simple to detect such cases,
> use of av_get_channel_layout_nb_channels is a good indicator, so I suggest we
> change this regardless.

No objection from me, and the changes look consistent.

Regards,
Marton Balint Dec. 10, 2016, 11:11 a.m. UTC | #2 
On Fri, 9 Dec 2016, Nicolas George wrote:

> L'octidi 18 frimaire, an CCXXV, Marton Balint a écrit :
>> Since the default in the libav fork is to only allow known layouts, making
>> unknown layouts allowed by default here can be a security risk for filters
>> directly merged from libav. However, usually it is simple to detect such cases,
>> use of av_get_channel_layout_nb_channels is a good indicator, so I suggest we
>> change this regardless.
>
> No objection from me, and the changes look consistent.
>

Thanks, applied.

Regards,
Marton
diff mbox

Patch

diff --git a/libavfilter/af_anull.c b/libavfilter/af_anull.c
index fff456e..1d0af45 100644
--- a/libavfilter/af_anull.c
+++ b/libavfilter/af_anull.c
@@ -46,7 +46,6 @@  static const AVFilterPad avfilter_af_anull_outputs[] = {
 AVFilter ff_af_anull = {
     .name          = "anull",
     .description   = NULL_IF_CONFIG_SMALL("Pass the source unchanged to the output."),
-    .query_formats = ff_query_formats_all,
     .inputs        = avfilter_af_anull_inputs,
     .outputs       = avfilter_af_anull_outputs,
 };
diff --git a/libavfilter/af_asyncts.c b/libavfilter/af_asyncts.c
index 22559a1..a33e0dd 100644
--- a/libavfilter/af_asyncts.c
+++ b/libavfilter/af_asyncts.c
@@ -317,6 +317,7 @@  AVFilter ff_af_asyncts = {
     .uninit      = uninit,
     .priv_size   = sizeof(ASyncContext),
     .priv_class  = &asyncts_class,
+    .query_formats = ff_query_formats_all_layouts,
     .inputs      = avfilter_af_asyncts_inputs,
     .outputs     = avfilter_af_asyncts_outputs,
 };
diff --git a/libavfilter/f_loop.c b/libavfilter/f_loop.c
index 69bfb10..5a32807 100644
--- a/libavfilter/f_loop.c
+++ b/libavfilter/f_loop.c
@@ -233,7 +233,6 @@  AVFilter ff_af_aloop = {
     .priv_size     = sizeof(LoopContext),
     .priv_class    = &aloop_class,
     .uninit        = auninit,
-    .query_formats = ff_query_formats_all,
     .inputs        = ainputs,
     .outputs       = aoutputs,
 };
diff --git a/libavfilter/f_metadata.c b/libavfilter/f_metadata.c
index 24deccf..1f613ec 100644
--- a/libavfilter/f_metadata.c
+++ b/libavfilter/f_metadata.c
@@ -373,7 +373,6 @@  AVFilter ff_af_ametadata = {
     .priv_class    = &ametadata_class,
     .init          = init,
     .uninit        = uninit,
-    .query_formats = ff_query_formats_all,
     .inputs        = ainputs,
     .outputs       = aoutputs,
     .flags         = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC,
diff --git a/libavfilter/f_sidedata.c b/libavfilter/f_sidedata.c
index 4863cc9..45d246b 100644
--- a/libavfilter/f_sidedata.c
+++ b/libavfilter/f_sidedata.c
@@ -139,7 +139,6 @@  AVFilter ff_af_asidedata = {
     .priv_size     = sizeof(SideDataContext),
     .priv_class    = &asidedata_class,
     .init          = init,
-    .query_formats = ff_query_formats_all,
     .inputs        = ainputs,
     .outputs       = aoutputs,
     .flags         = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC,
diff --git a/libavfilter/formats.c b/libavfilter/formats.c
index 20f45e3..840780d 100644
--- a/libavfilter/formats.c
+++ b/libavfilter/formats.c
@@ -596,12 +596,12 @@  static int default_query_formats_common(AVFilterContext *ctx,
 
 int ff_default_query_formats(AVFilterContext *ctx)
 {
-    return default_query_formats_common(ctx, ff_all_channel_layouts);
+    return default_query_formats_common(ctx, ff_all_channel_counts);
 }
 
-int ff_query_formats_all(AVFilterContext *ctx)
+int ff_query_formats_all_layouts(AVFilterContext *ctx)
 {
-    return default_query_formats_common(ctx, ff_all_channel_counts);
+    return default_query_formats_common(ctx, ff_all_channel_layouts);
 }
 
 /* internal functions for parsing audio format arguments */
diff --git a/libavfilter/formats.h b/libavfilter/formats.h
index 77981f5..870809b 100644
--- a/libavfilter/formats.h
+++ b/libavfilter/formats.h
@@ -186,15 +186,13 @@  void ff_channel_layouts_changeref(AVFilterChannelLayouts **oldref,
 av_warn_unused_result
 int ff_default_query_formats(AVFilterContext *ctx);
 
-/**
- * Set the formats list to all existing formats.
- * This function behaves like ff_default_query_formats(), except it also
- * accepts channel layouts with unknown disposition. It should only be used
- * with audio filters.
+ /**
+ * Set the formats list to all known channel layouts. This function behaves
+ * like ff_default_query_formats(), except it only accepts known channel
+ * layouts. It should only be used with audio filters.
  */
 av_warn_unused_result
-int ff_query_formats_all(AVFilterContext *ctx);
-
+int ff_query_formats_all_layouts(AVFilterContext *ctx);
 
 /**
  * Create a list of supported formats. This is intended for use in
diff --git a/libavfilter/split.c b/libavfilter/split.c
index 6cf4ab1..6630087 100644
--- a/libavfilter/split.c
+++ b/libavfilter/split.c
@@ -144,7 +144,6 @@  AVFilter ff_af_asplit = {
     .priv_class  = &asplit_class,
     .init        = split_init,
     .uninit      = split_uninit,
-    .query_formats = ff_query_formats_all,
     .inputs      = avfilter_af_asplit_inputs,
     .outputs     = NULL,
     .flags       = AVFILTER_FLAG_DYNAMIC_OUTPUTS,
diff --git a/libavfilter/trim.c b/libavfilter/trim.c
index 9daaeaf..1dbbabb 100644
--- a/libavfilter/trim.c
+++ b/libavfilter/trim.c
@@ -365,7 +365,6 @@  AVFilter ff_af_atrim = {
     .name        = "atrim",
     .description = NULL_IF_CONFIG_SMALL("Pick one continuous section from the input, drop the rest."),
     .init        = init,
-    .query_formats = ff_query_formats_all,
     .priv_size   = sizeof(TrimContext),
     .priv_class  = &atrim_class,
     .inputs      = atrim_inputs,