[FFmpeg-devel] [PATCH 3/4] avutil/frame: Reimplement av_frame_new_side_data() without size=0 special case

[wm4]

On Thu, 23 Feb 2017 15:19:31 +0100
Michael Niedermayer <michael at niedermayer.cc> wrote:

> The size 0 special case causes side data to be created which is
> different and a special case if for any reasons size = 0 is passed
> 
> Fixes: multiple runtime error: null pointer passed as argument 1, which is declared to never be null
> Fixes: 653/clusterfuzz-testcase-5773837415219200
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
> 
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavutil/frame.c | 55 +++++++++++++++++++++++++++++--------------------------
>  1 file changed, 29 insertions(+), 26 deletions(-)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 2913982e91..8811dcdcfe 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -26,6 +26,11 @@
>  #include "mem.h"
>  #include "samplefmt.h"
>  
> +
> +static AVFrameSideData *frame_new_side_data(AVFrame *frame,
> +                                            enum AVFrameSideDataType type,
> +                                            AVBufferRef *buf);
> +
>  MAKE_ACCESSORS(AVFrame, frame, int64_t, best_effort_timestamp)
>  MAKE_ACCESSORS(AVFrame, frame, int64_t, pkt_duration)
>  MAKE_ACCESSORS(AVFrame, frame, int64_t, pkt_pos)
> @@ -344,20 +349,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              }
>              memcpy(sd_dst->data, sd_src->data, sd_src->size);
>          } else {
> -            sd_dst = av_frame_new_side_data(dst, sd_src->type, 0);
> +            sd_dst = frame_new_side_data(dst, sd_src->type, av_buffer_ref(sd_src->buf));
>              if (!sd_dst) {
>                  wipe_side_data(dst);
>                  return AVERROR(ENOMEM);
>              }
> -            if (sd_src->buf) {
> -                sd_dst->buf = av_buffer_ref(sd_src->buf);
> -                if (!sd_dst->buf) {
> -                    wipe_side_data(dst);
> -                    return AVERROR(ENOMEM);
> -                }
> -                sd_dst->data = sd_dst->buf->data;
> -                sd_dst->size = sd_dst->buf->size;
> -            }
>          }
>          av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
>      }
> @@ -633,40 +629,47 @@ AVBufferRef *av_frame_get_plane_buffer(AVFrame *frame, int plane)
>      return NULL;
>  }
>  
> -AVFrameSideData *av_frame_new_side_data(AVFrame *frame,
> -                                        enum AVFrameSideDataType type,
> -                                        int size)
> +static AVFrameSideData *frame_new_side_data(AVFrame *frame,
> +                                            enum AVFrameSideDataType type,
> +                                            AVBufferRef *buf)
>  {
>      AVFrameSideData *ret, **tmp;
>  
> -    if (frame->nb_side_data > INT_MAX / sizeof(*frame->side_data) - 1)
> +    if (!buf)
>          return NULL;
>  
> +    if (frame->nb_side_data > INT_MAX / sizeof(*frame->side_data) - 1)
> +        goto fail;
> +
>      tmp = av_realloc(frame->side_data,
>                       (frame->nb_side_data + 1) * sizeof(*frame->side_data));
>      if (!tmp)
> -        return NULL;
> +        goto fail;
>      frame->side_data = tmp;
>  
>      ret = av_mallocz(sizeof(*ret));
>      if (!ret)
> -        return NULL;
> -
> -    if (size > 0) {
> -        ret->buf = av_buffer_alloc(size);
> -        if (!ret->buf) {
> -            av_freep(&ret);
> -            return NULL;
> -        }
> +        goto fail;
>  
> -        ret->data = ret->buf->data;
> -        ret->size = size;
> -    }
> +    ret->buf = buf;
> +    ret->data = ret->buf->data;
> +    ret->size = buf->size;
>      ret->type = type;
>  
>      frame->side_data[frame->nb_side_data++] = ret;
>  
>      return ret;
> +fail:
> +    av_buffer_unref(&buf);
> +    return NULL;
> +}
> +
> +AVFrameSideData *av_frame_new_side_data(AVFrame *frame,
> +                                        enum AVFrameSideDataType type,
> +                                        int size)
> +{
> +
> +    return frame_new_side_data(frame, type, av_buffer_alloc(size));
>  }
>  
>  AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,

Why not restore Libav's version, which doesn't seem to need all this?