diff mbox series

[FFmpeg-devel,14/19] avfilter/af_amerge: Fix segfault upon allocation failure

Message ID 20200825140927.16433-14-andreas.rheinhardt@gmail.com
State Accepted
Commit 8f2c1f2cbe77685435362f1940b637a6c3ff1934
Headers show
Series [FFmpeg-devel,01/19] avfilter/avfilter: Fix indentation | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Aug. 25, 2020, 2:09 p.m. UTC
The amerge filter uses a variable number of inpads and allocates them
in its init function; if all goes well, the number of inpads coincides
with a number stored in the filter's private context. Yet if allocating a
subsequent inpad fails, the uninit function nevertheless uses the number
stored in the private context to determine the number of inpads to free
and not the AVFilterContext's nb_inputs. This will lead to an access
beyond the end of the allocated AVFilterContext.input_pads array and
an invalid free.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavfilter/af_amerge.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Paul B Mahol Aug. 30, 2020, 7:25 p.m. UTC | #1
On 8/25/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
> The amerge filter uses a variable number of inpads and allocates them
> in its init function; if all goes well, the number of inpads coincides
> with a number stored in the filter's private context. Yet if allocating a
> subsequent inpad fails, the uninit function nevertheless uses the number
> stored in the private context to determine the number of inpads to free
> and not the AVFilterContext's nb_inputs. This will lead to an access
> beyond the end of the allocated AVFilterContext.input_pads array and
> an invalid free.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---

LGTM, but might wait for Nicolas.

>  libavfilter/af_amerge.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/libavfilter/af_amerge.c b/libavfilter/af_amerge.c
> index ca94a224af..93f6f17d22 100644
> --- a/libavfilter/af_amerge.c
> +++ b/libavfilter/af_amerge.c
> @@ -58,13 +58,10 @@ AVFILTER_DEFINE_CLASS(amerge);
>  static av_cold void uninit(AVFilterContext *ctx)
>  {
>      AMergeContext *s = ctx->priv;
> -    int i;
>
> -    for (i = 0; i < s->nb_inputs; i++) {
> -        if (ctx->input_pads)
> -            av_freep(&ctx->input_pads[i].name);
> -    }
>      av_freep(&s->in);
> +    for (unsigned i = 0; i < ctx->nb_inputs; i++)
> +        av_freep(&ctx->input_pads[i].name);
>  }
>
>  static int query_formats(AVFilterContext *ctx)
> --
> 2.20.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavfilter/af_amerge.c b/libavfilter/af_amerge.c
index ca94a224af..93f6f17d22 100644
--- a/libavfilter/af_amerge.c
+++ b/libavfilter/af_amerge.c
@@ -58,13 +58,10 @@  AVFILTER_DEFINE_CLASS(amerge);
 static av_cold void uninit(AVFilterContext *ctx)
 {
     AMergeContext *s = ctx->priv;
-    int i;
 
-    for (i = 0; i < s->nb_inputs; i++) {
-        if (ctx->input_pads)
-            av_freep(&ctx->input_pads[i].name);
-    }
     av_freep(&s->in);
+    for (unsigned i = 0; i < ctx->nb_inputs; i++)
+        av_freep(&ctx->input_pads[i].name);
 }
 
 static int query_formats(AVFilterContext *ctx)