On Sat, 27 Feb 2016 13:01:40 +0100
Reimar Döffinger <reimar.doeffin...@gmx.de> wrote:

> Check that the required plane pointers and only
> those are set up.
> Currently does not enforce anything for the palette
> pointer of pseudopal formats as I am unsure about the
> requirements.
> 
> Signed-off-by: Reimar Döffinger <reimar.doeffin...@gmx.de>
> ---
>  doc/APIchanges       |  6 ++++++
>  libavcodec/utils.c   | 27 +++++++++++++++++++++++++++
>  libavcodec/version.h |  2 +-
>  libavutil/frame.h    |  3 +++
>  4 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 4e952a8..ee407da 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,12 @@ libavutil:     2015-08-28
>  
>  API changes, most recent first:
>  
> +2016-02-xx - xxxxxxx - lavc 57.25.101
> +  Validate AVFrame returned by get_buffer2 to have required
> +  planes not NULL and unused planes set to NULL as crashes
> +  and buffer overflow are possible with certain streams if
> +  that is not the case.
> +
>  2016-xx-xx - lavc 57.25.0 - avcodec.h
>    Add AVCodecContext.hw_frames_ctx.
>  
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index af21cdd..1967f03 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -853,6 +853,31 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame 
> *frame)
>      return ff_init_buffer_info(avctx, frame);
>  }
>  
> +static void validate_avframe_allocation(AVCodecContext *avctx, AVFrame 
> *frame)
> +{
> +    if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
> +        int i;
> +        int num_planes = av_pix_fmt_count_planes(frame->format);
> +        const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
> +        int flags = desc ? desc->flags : 0;
> +        if (num_planes == 1 && (flags & AV_PIX_FMT_FLAG_PAL))
> +            num_planes = 2;
> +        for (i = 0; i < num_planes; i++) {
> +            av_assert0(frame->data[i]);
> +        }
> +        // TODO: what are the exact requirements on pseudopal formats for 
> get_buffer2?
> +        if (num_planes == 1 && (flags & AV_PIX_FMT_FLAG_PSEUDOPAL))
> +            num_planes = 2;

This should be correct. Not allocating a palette for unpaletted but
"pseudopal" formats can cause random crashes, as all code within FFmpeg
seems to assume that the palette has been allocated. (An utter
abomination, if you ask me.) At the same time, pixdesc will indicate
that paletted formats have only 1 plane, but 2 data pointers.

> +        // for formats without data like hwaccel allow
> +        // pointers to be non-NULL.
> +        for (i = num_planes; num_planes > 0 && i < 
> FF_ARRAY_ELEMS(frame->data); i++) {
> +            if (frame->data[i])
> +                av_log(avctx, AV_LOG_ERROR, "Buffer returned by 
> get_buffer2() did not zero unused plane pointers\n");
> +            frame->data[i] = NULL;
> +        }
> +    }
> +}
> +
>  static int get_buffer_internal(AVCodecContext *avctx, AVFrame *frame, int 
> flags)
>  {
>      const AVHWAccel *hwaccel = avctx->hwaccel;
> @@ -885,6 +910,8 @@ static int get_buffer_internal(AVCodecContext *avctx, 
> AVFrame *frame, int flags)
>          avctx->sw_pix_fmt = avctx->pix_fmt;
>  
>      ret = avctx->get_buffer2(avctx, frame, flags);
> +    if (ret >= 0)
> +        validate_avframe_allocation(avctx, frame);
>  
>  end:
>      if (avctx->codec_type == AVMEDIA_TYPE_VIDEO && !override_dimensions) {
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 0856b19..c937eaf 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>  
>  #define LIBAVCODEC_VERSION_MAJOR  57
>  #define LIBAVCODEC_VERSION_MINOR  25
> -#define LIBAVCODEC_VERSION_MICRO 101
> +#define LIBAVCODEC_VERSION_MICRO 102
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>                                                 LIBAVCODEC_VERSION_MINOR, \
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 56001a8..76a8123 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -187,6 +187,9 @@ typedef struct AVFrame {
>       * see avcodec_align_dimensions2(). Some filters and swscale can read
>       * up to 16 bytes beyond the planes, if these filters are to be used,
>       * then 16 extra bytes must be allocated.
> +     *
> +     * NOTE: Except for hwaccel formats, pointers not needed by the format
> +     * MUST be set to NULL.
>       */
>      uint8_t *data[AV_NUM_DATA_POINTERS];
>  

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to