| Message ID | 20171028161417.15172-1-onemda@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Nack. Le septidi 7 brumaire, an CCXXVI, Paul B Mahol a écrit : > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavfilter/vf_tile.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/libavfilter/vf_tile.c b/libavfilter/vf_tile.c > index 87e0b940cf..5752ca080e 100644 > --- a/libavfilter/vf_tile.c > +++ b/libavfilter/vf_tile.c > @@ -23,6 +23,7 @@ > * tile video filter > */ > > +#include "libavutil/imgutils.h" > #include "libavutil/opt.h" > #include "libavutil/pixdesc.h" > #include "avfilter.h" > @@ -44,8 +45,6 @@ typedef struct TileContext { > uint8_t rgba_color[4]; > } TileContext; > > -#define REASONABLE_SIZE 1024 > - > #define OFFSET(x) offsetof(TileContext, x) > #define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM > > @@ -68,12 +67,6 @@ static av_cold int init(AVFilterContext *ctx) > { > TileContext *tile = ctx->priv; > > - if (tile->w > REASONABLE_SIZE || tile->h > REASONABLE_SIZE) { > - av_log(ctx, AV_LOG_ERROR, "Tile size %ux%u is insane.\n", > - tile->w, tile->h); > - return AVERROR(EINVAL); > - } > - > if (tile->nb_frames == 0) { > tile->nb_frames = tile->w * tile->h; This multiplication still overflows. The problem is caught later because a tile size that results in an overflow will also result in a too large image, but this is fragile design, since the values that are being tested has no direct relation with the value that is being invalid. And that is not all: const unsigned total_margin_w = (tile->w - 1) * tile->padding + 2*tile->margin; const unsigned total_margin_h = (tile->h - 1) * tile->padding + 2*tile->margin; These computations will also overflow with a large tile size. This would result in tile happily drawing beyond the boundaries of the image. It does not actually happens because the frame pool is inited using av_image_check_size() rather than av_image_check_size2() and thus uses a stride that is larger than necessary and causes the image to seem too large. But once this is fixed, it will happen. I will not accept bugs added just because they are hidden by other bugs. That would be a highway to catastrophe. If you want this change to be accepted, you need to perform all the overflow check correctly (or prove that they are not needed), not throw random checks around in the hope they catch problems. > } else if (tile->nb_frames > tile->w * tile->h) { > @@ -116,7 +109,7 @@ static int config_props(AVFilterLink *outlink) > ff_draw_init(&tile->draw, inlink->format, 0); > ff_draw_color(&tile->draw, &tile->blank, tile->rgba_color); > > - return 0; > + return av_image_check_size2(outlink->w, outlink->h, INT64_MAX, inlink->format, 0, ctx); > } > > static void get_current_tile_pos(AVFilterContext *ctx, unsigned *x, unsigned *y) > @@ -142,6 +135,7 @@ static void draw_blank_frame(AVFilterContext *ctx, AVFrame *out_buf) > x0, y0, inlink->w, inlink->h); > tile->current++; > } > + > static int end_last_frame(AVFilterContext *ctx) > { > TileContext *tile = ctx->priv;
diff --git a/libavfilter/vf_tile.c b/libavfilter/vf_tile.c index 87e0b940cf..5752ca080e 100644 --- a/libavfilter/vf_tile.c +++ b/libavfilter/vf_tile.c @@ -23,6 +23,7 @@ * tile video filter */ +#include "libavutil/imgutils.h" #include "libavutil/opt.h" #include "libavutil/pixdesc.h" #include "avfilter.h" @@ -44,8 +45,6 @@ typedef struct TileContext { uint8_t rgba_color[4]; } TileContext; -#define REASONABLE_SIZE 1024 - #define OFFSET(x) offsetof(TileContext, x) #define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM @@ -68,12 +67,6 @@ static av_cold int init(AVFilterContext *ctx) { TileContext *tile = ctx->priv; - if (tile->w > REASONABLE_SIZE || tile->h > REASONABLE_SIZE) { - av_log(ctx, AV_LOG_ERROR, "Tile size %ux%u is insane.\n", - tile->w, tile->h); - return AVERROR(EINVAL); - } - if (tile->nb_frames == 0) { tile->nb_frames = tile->w * tile->h; } else if (tile->nb_frames > tile->w * tile->h) { @@ -116,7 +109,7 @@ static int config_props(AVFilterLink *outlink) ff_draw_init(&tile->draw, inlink->format, 0); ff_draw_color(&tile->draw, &tile->blank, tile->rgba_color); - return 0; + return av_image_check_size2(outlink->w, outlink->h, INT64_MAX, inlink->format, 0, ctx); } static void get_current_tile_pos(AVFilterContext *ctx, unsigned *x, unsigned *y) @@ -142,6 +135,7 @@ static void draw_blank_frame(AVFilterContext *ctx, AVFrame *out_buf) x0, y0, inlink->w, inlink->h); tile->current++; } + static int end_last_frame(AVFilterContext *ctx) { TileContext *tile = ctx->priv;
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavfilter/vf_tile.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)