[FFmpeg-devel] [PATCH 3/4] avfilter/vf_paletteuse: Fix potential double-free of AVFrame

[Paul B Mahol]

LGTM

On 1/27/20, Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:
> apply_palette() would free an AVFrame given to it only via an AVFrame *
> (and not via AVFrame **) in three of its four exists (namely in the
> normal path and in two error paths). So upon error the caller has no way
> to know whether the frame has already been freed or not;
> load_apply_palette(), the only caller, opted to free the frame in this
> scenario.
>
> This commit changes this by making apply_palette not freeing the frame
> at all, which is left to load_apply_palette().
>
> Fixes Coverity issue #1452434.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavfilter/vf_paletteuse.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/libavfilter/vf_paletteuse.c b/libavfilter/vf_paletteuse.c
> index ed128813d6..255c9d79e3 100644
> --- a/libavfilter/vf_paletteuse.c
> +++ b/libavfilter/vf_paletteuse.c
> @@ -903,7 +903,6 @@ static int apply_palette(AVFilterLink *inlink, AVFrame
> *in, AVFrame **outf)
>
>      AVFrame *out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
>      if (!out) {
> -        av_frame_free(&in);
>          *outf = NULL;
>          return AVERROR(ENOMEM);
>      }
> @@ -916,7 +915,6 @@ static int apply_palette(AVFilterLink *inlink, AVFrame
> *in, AVFrame **outf)
>      if (av_frame_ref(s->last_in, in) < 0 ||
>          av_frame_ref(s->last_out, out) < 0 ||
>          av_frame_make_writable(s->last_in) < 0) {
> -        av_frame_free(&in);
>          av_frame_free(&out);
>          *outf = NULL;
>          return AVERROR(ENOMEM);
> @@ -934,7 +932,6 @@ static int apply_palette(AVFilterLink *inlink, AVFrame
> *in, AVFrame **outf)
>      memcpy(out->data[1], s->palette, AVPALETTE_SIZE);
>      if (s->calc_mean_err)
>          debug_mean_error(s, in, out, inlink->frame_count_out);
> -    av_frame_free(&in);
>      *outf = out;
>      return 0;
>  }
> @@ -1023,20 +1020,17 @@ static int load_apply_palette(FFFrameSync *fs)
>      if (ret < 0)
>          return ret;
>      if (!master || !second) {
> -        ret = AVERROR_BUG;
> -        goto error;
> +        av_frame_free(&master);
> +        return AVERROR_BUG;
>      }
>      if (!s->palette_loaded) {
>          load_palette(s, second);
>      }
>      ret = apply_palette(inlink, master, &out);
> +    av_frame_free(&master);
>      if (ret < 0)
> -        goto error;
> +        return ret;
>      return ff_filter_frame(ctx->outputs[0], out);
> -
> -error:
> -    av_frame_free(&master);
> -    return ret;
>  }
>
>  #define DEFINE_SET_FRAME(color_search, name, value)
>     \
> --
> 2.20.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".