diff mbox series

[FFmpeg-devel,02/26] avfilter/af_agate: Honour query_formats API, fix segfault

Message ID AM7PR03MB6660756C3D63CBB2A5EF72398FA19@AM7PR03MB6660.eurprd03.prod.outlook.com
State Accepted
Commit 80d32e0f7e9dd3263b64cc0cc4b744de212c61fa
Headers show
Series [FFmpeg-devel,01/26] avfilter/af_afade: Remove redundant checks and assignments
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Andreas Rheinhardt Sept. 21, 2021, 6:27 a.m. UTC
The sidechaingate filter wants its main input and its (only) output
to have the same channel layout and number of channels; yet it does
not link them in its query_formats callback. Instead it sets the
outlink to only accept the first offered choice for the main input's
channel layout and then sets both inputs to independently accept
any channel counts. The config_output callback then overwrote the
outlink's channel layout and channels properties with the main input's,
even though they may differ in case the first offered choice for
the main input's channel layout turns out not to be the final one.

Consider e.g. the following filtergraph:
[in]aformat=channel_layouts=mono,aformat=channel_layouts=stereo|mono[out];\
[out][in2]sidechaingate,stereotools
The two aformats ensure that the first offered channel layout (stereo)
will not be chosen for the input; yet it is the only offered channel
layout for the output of sidechaingate and will therefore be chosen
by the query_formats framework. Because the sidechaingate outputs
interleaved doubles which stereotools expects the output of
sidechaingate appears to be suitable as input for stereotools without
further conversions. Yet stereotools actually only receives a mono frame
and therefore overreads its input buffer which leads to segfaults;
it can also lead to heap corruption because there can be writes beyond
the end of the buffer, too.

Fix this by linking the channel layouts of the main input and the output
in query_formats and remove the code overwriting it in config_output.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/af_agate.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

Comments

Paul B Mahol Sept. 21, 2021, 6:59 a.m. UTC | #1
lgtm
diff mbox series

Patch

diff --git a/libavfilter/af_agate.c b/libavfilter/af_agate.c
index 284880833a..8e5cca4228 100644
--- a/libavfilter/af_agate.c
+++ b/libavfilter/af_agate.c
@@ -332,29 +332,19 @@  static int activate(AVFilterContext *ctx)
 
 static int scquery_formats(AVFilterContext *ctx)
 {
-    AVFilterChannelLayouts *layouts = NULL;
     static const enum AVSampleFormat sample_fmts[] = {
         AV_SAMPLE_FMT_DBL,
         AV_SAMPLE_FMT_NONE
     };
-    int ret, i;
-
-    if (!ctx->inputs[0]->incfg.channel_layouts ||
-        !ctx->inputs[0]->incfg.channel_layouts->nb_channel_layouts) {
-        av_log(ctx, AV_LOG_WARNING,
-               "No channel layout for input 1\n");
-            return AVERROR(EAGAIN);
-    }
-
-    if ((ret = ff_add_channel_layout(&layouts, ctx->inputs[0]->incfg.channel_layouts->channel_layouts[0])) < 0 ||
-        (ret = ff_channel_layouts_ref(layouts, &ctx->outputs[0]->incfg.channel_layouts)) < 0)
+    int ret = ff_channel_layouts_ref(ff_all_channel_counts(),
+                                     &ctx->inputs[1]->outcfg.channel_layouts);
+    if (ret < 0)
         return ret;
 
-    for (i = 0; i < 2; i++) {
-        layouts = ff_all_channel_counts();
-        if ((ret = ff_channel_layouts_ref(layouts, &ctx->inputs[i]->outcfg.channel_layouts)) < 0)
-            return ret;
-    }
+    /* This will link the channel properties of the main input and the output;
+     * it won't touch the second input as its channel_layouts is already set. */
+    if ((ret = ff_set_common_all_channel_counts(ctx)) < 0)
+        return ret;
 
     if ((ret = ff_set_common_formats_from_list(ctx, sample_fmts)) < 0)
         return ret;
@@ -377,8 +367,6 @@  static int scconfig_output(AVFilterLink *outlink)
 
     outlink->sample_rate = ctx->inputs[0]->sample_rate;
     outlink->time_base   = ctx->inputs[0]->time_base;
-    outlink->channel_layout = ctx->inputs[0]->channel_layout;
-    outlink->channels = ctx->inputs[0]->channels;
 
     s->fifo[0] = av_audio_fifo_alloc(ctx->inputs[0]->format, ctx->inputs[0]->channels, 1024);
     s->fifo[1] = av_audio_fifo_alloc(ctx->inputs[1]->format, ctx->inputs[1]->channels, 1024);