diff mbox series

[FFmpeg-devel,11/19] avfilter/af_aiir: Fix segfault and leak upon allocation failure

Message ID 20200825140927.16433-11-andreas.rheinhardt@gmail.com
State Accepted
Commit 97b1a2c564e0d4dbf8573b4647ae110a75238db3
Headers show
Series [FFmpeg-devel,01/19] avfilter/avfilter: Fix indentation
Related show

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 aiir filter adds output pads in its init function. Each of these
output pads had a name which was allocated and to be freed in the uninit
function. Given that the aiir filter has between one and two outputs,
one output pad's name was freed unconditionally and a second was freed
conditionally.

Yet if adding output pads fails, there are no output pads at all and
trying to free a nonexistent pad's name will lead to a segfault.

Furthermore, if the name could be successfully allocated, yet adding the
new pad fails, the name would leak.

This commit fixes this by not allocating the pads' names at all any
more: They are constant anyway. This allows to remove the code to free
them and hence fixes the aforementioned bugs.

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

Comments

Paul B Mahol Aug. 26, 2020, 8:14 p.m. UTC | #1
On 8/25/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
> The aiir filter adds output pads in its init function. Each of these
> output pads had a name which was allocated and to be freed in the uninit
> function. Given that the aiir filter has between one and two outputs,
> one output pad's name was freed unconditionally and a second was freed
> conditionally.
>
> Yet if adding output pads fails, there are no output pads at all and
> trying to free a nonexistent pad's name will lead to a segfault.
>
> Furthermore, if the name could be successfully allocated, yet adding the
> new pad fails, the name would leak.
>
> This commit fixes this by not allocating the pads' names at all any
> more: They are constant anyway. This allows to remove the code to free
> them and hence fixes the aforementioned bugs.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavfilter/af_aiir.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
>

LGTM

> diff --git a/libavfilter/af_aiir.c b/libavfilter/af_aiir.c
> index bc31e5141e..3df25b4d9b 100644
> --- a/libavfilter/af_aiir.c
> +++ b/libavfilter/af_aiir.c
> @@ -1159,26 +1159,21 @@ static av_cold int init(AVFilterContext *ctx)
>      }
>
>      pad = (AVFilterPad){
> -        .name         = av_strdup("default"),
> +        .name         = "default",
>          .type         = AVMEDIA_TYPE_AUDIO,
>          .config_props = config_output,
>      };
>
> -    if (!pad.name)
> -        return AVERROR(ENOMEM);
> -
>      ret = ff_insert_outpad(ctx, 0, &pad);
>      if (ret < 0)
>          return ret;
>
>      if (s->response) {
>          vpad = (AVFilterPad){
> -            .name         = av_strdup("filter_response"),
> +            .name         = "filter_response",
>              .type         = AVMEDIA_TYPE_VIDEO,
>              .config_props = config_video,
>          };
> -        if (!vpad.name)
> -            return AVERROR(ENOMEM);
>
>          ret = ff_insert_outpad(ctx, 1, &vpad);
>          if (ret < 0)
> @@ -1205,9 +1200,6 @@ static av_cold void uninit(AVFilterContext *ctx)
>      }
>      av_freep(&s->iir);
>
> -    av_freep(&ctx->output_pads[0].name);
> -    if (s->response)
> -        av_freep(&ctx->output_pads[1].name);
>      av_frame_free(&s->video);
>  }
>
> --
> 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_aiir.c b/libavfilter/af_aiir.c
index bc31e5141e..3df25b4d9b 100644
--- a/libavfilter/af_aiir.c
+++ b/libavfilter/af_aiir.c
@@ -1159,26 +1159,21 @@  static av_cold int init(AVFilterContext *ctx)
     }
 
     pad = (AVFilterPad){
-        .name         = av_strdup("default"),
+        .name         = "default",
         .type         = AVMEDIA_TYPE_AUDIO,
         .config_props = config_output,
     };
 
-    if (!pad.name)
-        return AVERROR(ENOMEM);
-
     ret = ff_insert_outpad(ctx, 0, &pad);
     if (ret < 0)
         return ret;
 
     if (s->response) {
         vpad = (AVFilterPad){
-            .name         = av_strdup("filter_response"),
+            .name         = "filter_response",
             .type         = AVMEDIA_TYPE_VIDEO,
             .config_props = config_video,
         };
-        if (!vpad.name)
-            return AVERROR(ENOMEM);
 
         ret = ff_insert_outpad(ctx, 1, &vpad);
         if (ret < 0)
@@ -1205,9 +1200,6 @@  static av_cold void uninit(AVFilterContext *ctx)
     }
     av_freep(&s->iir);
 
-    av_freep(&ctx->output_pads[0].name);
-    if (s->response)
-        av_freep(&ctx->output_pads[1].name);
     av_frame_free(&s->video);
 }