[FFmpeg-devel] [PATCH 3/3] avutil/frame: Check at some random points that the AVFrame structure matches what it should be.

[Nicolas George]

Le septidi 27 frimaire, an CCXXII, Michael Niedermayer a écrit :
> If a mismatch is detected a error is printed once and the program aborts.
> As a special exception at assertion levels below 2 an apparently zeroed structure
> is accepted with just an error
> 
> This detects cases where avframes have been allocated by some method different than
> libavutils. It also detects if they have been allocated by a different libavutil
> version, which can happen if 2 different libavutil.so get loaded.
> Previously such mismatches would lead to random problems or crashes, this aborts
> with a clear error message.
> 
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavutil/frame.c |   39 ++++++++++++++++++++++++++++-----------
>  1 file changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 013e439..06964eb 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -27,17 +27,32 @@
>  #include "mem.h"
>  #include "samplefmt.h"
>  
> -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)
> -MAKE_ACCESSORS(AVFrame, frame, int64_t, channel_layout)
> -MAKE_ACCESSORS(AVFrame, frame, int,     channels)
> -MAKE_ACCESSORS(AVFrame, frame, int,     sample_rate)
> -MAKE_ACCESSORS(AVFrame, frame, AVDictionary *, metadata)
> -MAKE_ACCESSORS(AVFrame, frame, int,     decode_error_flags)
> -MAKE_ACCESSORS(AVFrame, frame, int,     pkt_size)
> -MAKE_ACCESSORS(AVFrame, frame, enum AVColorSpace, colorspace)
> -MAKE_ACCESSORS(AVFrame, frame, enum AVColorRange, color_range)
> +static void check_frame_struct_version(const AVFrame *frame)
> +{

> +    if (frame->struct_version != LIBAVUTIL_VERSION_MAJOR + (sizeof(AVFrame)<<8) + ((uint64_t)MKTAG('A','V','F','r')<<32)) {

I would suggest to make this constant at least a file-wide macro in patch
#1.

> +        static int warned;
> +        if (!warned)
> +            av_log(NULL, AV_LOG_FATAL,
> +                   "Corrupted AVFrame or wrong structure detected.\n"
> +                   "Check that you arent loading 2 different libavutil versions at the same time\n"
> +                   "And that you use av_frame_alloc() and not malloc or memset on an AVFrame\n");
> +        warned = 1;
> +        av_assert2(0);
> +        av_assert0(!frame->struct_version);
> +    }
> +}
> +
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, int64_t, best_effort_timestamp, check_frame_struct_version)
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, int64_t, pkt_duration         , check_frame_struct_version)
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, int64_t, pkt_pos              , check_frame_struct_version)
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, int64_t, channel_layout       , check_frame_struct_version)
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, int,     channels             , check_frame_struct_version)
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, int,     sample_rate          , check_frame_struct_version)
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, AVDictionary *, metadata      , check_frame_struct_version)
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, int,     decode_error_flags   , check_frame_struct_version)
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, int,     pkt_size             , check_frame_struct_version)
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, enum AVColorSpace, colorspace , check_frame_struct_version)
> +MAKE_CHECKED_ACCESSORS(AVFrame, frame, enum AVColorRange, color_range, check_frame_struct_version)
>  
>  #define CHECK_CHANNELS_CONSISTENCY(frame) \
>      av_assert2(!(frame)->channel_layout || \
> @@ -408,6 +423,8 @@ int av_frame_make_writable(AVFrame *frame)
>      AVFrame tmp;
>      int ret;
>  
> +    check_frame_struct_version(frame);
> +
>      if (!frame->buf[0])
>          return AVERROR(EINVAL);
>  

Apart from that, I do not have a better suggestion to handle the current
situation, so I have no objection.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131218/b931be74/attachment.asc>