On Thu, 8 Dec 2016 19:30:25 +0100
Michael Niedermayer <mich...@niedermayer.cc> wrote:
> TODO: split into 2 patches (one per lib), docs & bump
>
> This allows preventing some OOM and "slow decoding" cases by limiting the
> maximum resolution
> this may be useful to avoid fuzzers getting stuck in boring cases
>
> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> ---
> libavcodec/avcodec.h | 8 ++++++++
> libavcodec/options_table.h | 1 +
> libavutil/imgutils.c | 22 ++++++++++++++++++----
> tests/ref/fate/api-mjpeg-codec-param | 2 ++
> tests/ref/fate/api-png-codec-param | 2 ++
> 5 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 7ac2adaf66..81052b10ef 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3570,6 +3570,14 @@ typedef struct AVCodecContext {
> */
> int trailing_padding;
>
> + /**
> + * The number of pixels per image to maximally accept.
> + *
> + * - decoding: set by user
> + * - encoding: unused
> + */
> + int max_pixels;
> +
> } AVCodecContext;
>
> AVRational av_codec_get_pkt_timebase (const AVCodecContext *avctx);
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index ee79859953..2f5eb252fe 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -570,6 +570,7 @@ static const AVOption avcodec_options[] = {
> {"codec_whitelist", "List of decoders that are allowed to be used",
> OFFSET(codec_whitelist), AV_OPT_TYPE_STRING, { .str = NULL }, CHAR_MIN,
> CHAR_MAX, A|V|S|D },
> {"pixel_format", "set pixel format", OFFSET(pix_fmt), AV_OPT_TYPE_PIXEL_FMT,
> {.i64=AV_PIX_FMT_NONE}, -1, INT_MAX, 0 },
> {"video_size", "set video size", OFFSET(width), AV_OPT_TYPE_IMAGE_SIZE,
> {.str=NULL}, 0, INT_MAX, 0 },
> +{"max_pixels", "Maximum number of pixels", OFFSET(max_pixels),
> AV_OPT_TYPE_INT, {.i64 = INT_MAX }, 0, INT_MAX, A|V|S|D },
> {NULL},
> };
>
> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> index 37808e53d0..7c42ec7e17 100644
> --- a/libavutil/imgutils.c
> +++ b/libavutil/imgutils.c
> @@ -30,6 +30,7 @@
> #include "mathematics.h"
> #include "pixdesc.h"
> #include "rational.h"
> +#include "opt.h"
>
> void av_image_fill_max_pixsteps(int max_pixsteps[4], int
> max_pixstep_comps[4],
> const AVPixFmtDescriptor *pixdesc)
> @@ -256,11 +257,24 @@ int av_image_check_size(unsigned int w, unsigned int h,
> int log_offset, void *lo
> .log_ctx = log_ctx,
> };
>
> - if ((int)w>0 && (int)h>0 && (w+128)*(uint64_t)(h+128) < INT_MAX/8)
> - return 0;
> + if ((int)w<=0 || (int)h<=0 || (w+128)*(uint64_t)(h+128) >= INT_MAX/8) {
> + av_log(&imgutils, AV_LOG_ERROR, "Picture size %ux%u is invalid\n",
> w, h);
> + return AVERROR(EINVAL);
> + }
>
> - av_log(&imgutils, AV_LOG_ERROR, "Picture size %ux%u is invalid\n", w, h);
> - return AVERROR(EINVAL);
> + if (log_ctx) {
> + int64_t max = INT_MAX;
> + if (av_opt_get_int(log_ctx, "max_pixels", AV_OPT_SEARCH_CHILDREN,
> &max) >= 0) {
IMHO terrible implementation. Just add a new function that takes an
honest argument?
> + if (w*h > max) {
> + av_log(&imgutils, AV_LOG_ERROR,
> + "Picture size %ux%u exceeds specified max pixel count
> %"PRId64"\n",
> + w, h, max);
> + return AVERROR(EINVAL);
> + }
> + }
> + }
> +
> + return 0;
> }
>
> int av_image_check_sar(unsigned int w, unsigned int h, AVRational sar)
> diff --git a/tests/ref/fate/api-mjpeg-codec-param
> b/tests/ref/fate/api-mjpeg-codec-param
> index c67d1b1ab0..08313adab3 100644
> --- a/tests/ref/fate/api-mjpeg-codec-param
> +++ b/tests/ref/fate/api-mjpeg-codec-param
> @@ -155,6 +155,7 @@ stream=0, decode=0
> codec_whitelist=
> pixel_format=yuvj422p
> video_size=400x225
> + max_pixels=2147483647
> stream=0, decode=1
> b=0
> ab=0
> @@ -312,3 +313,4 @@ stream=0, decode=1
> codec_whitelist=
> pixel_format=yuvj422p
> video_size=400x225
> + max_pixels=2147483647
> diff --git a/tests/ref/fate/api-png-codec-param
> b/tests/ref/fate/api-png-codec-param
> index bd53441894..7a9a921461 100644
> --- a/tests/ref/fate/api-png-codec-param
> +++ b/tests/ref/fate/api-png-codec-param
> @@ -155,6 +155,7 @@ stream=0, decode=0
> codec_whitelist=
> pixel_format=rgba
> video_size=128x128
> + max_pixels=2147483647
> stream=0, decode=1
> b=0
> ab=0
> @@ -312,3 +313,4 @@ stream=0, decode=1
> codec_whitelist=
> pixel_format=rgba
> video_size=128x128
> + max_pixels=2147483647
In general I'd rather have the current pixel limit _removed_. It causes
problems with processing high-res images.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
