From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id B1D624B402 for ; Wed, 10 Jul 2024 15:28:20 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A149668DB40; Wed, 10 Jul 2024 18:28:17 +0300 (EEST) Received: from mail-ed1-f53.google.com (mail-ed1-f53.google.com [209.85.208.53]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id AD89D68D892 for ; Wed, 10 Jul 2024 18:28:11 +0300 (EEST) Received: by mail-ed1-f53.google.com with SMTP id 4fb4d7f45d1cf-5956fbe6d0dso2424071a12.1 for ; Wed, 10 Jul 2024 08:28:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1720625291; x=1721230091; darn=ffmpeg.org; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=w6hX7cSkqhtTXqfFsO0qiUvW57iN/vLdJGFzx/O1EwQ=; b=LTtN2pzi14ol6eGs5EKPKP82CkMPz6aODR+7K51WclZmeHtz0C4ubVZdLAvHvd2Shv b+OJKzPYck9fA0KPqz2KYLk4LT2z6O/N1dZYoVJII/Hu8VmN580POE4H9XJUv+g8XdcA v71iD1m/NphvznBlDfHSrXJ5MICny+XYy+UU4v+82GFU/YvG5C/K0cajBhnieDWrCyjH ZATDVOUr/OSXEEqu39Y2OrX+Z8dHmD+2enJ//yRPYT148TSsUSr0dPI47D3V4b2Vk39E CLylLt0X3UdBhfx05Ke1g7x48zhnbxX2rTEOKRbHLo3oB1SK8OOTDddmv5zc9CulIChB H/DA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720625291; x=1721230091; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=w6hX7cSkqhtTXqfFsO0qiUvW57iN/vLdJGFzx/O1EwQ=; b=mwBd4jL540mijKdqlLiGbN9dx7S4KJaNSNkRVN15zyiQ4m2uQ1YqW5w6DAmp7XiYtz hYIQPK2oRB9vrfE0G7jlxurnkQg//acVEpM6lgeokyG/ytwZOCSO+9Ej9x91bsQDsyU3 2ZG3sPRqS8tuXqLX1iFVo8MIh/xf5GfC3JAlr0iqBIL6rME5Rb8S7wbZPBPWYFl+LErJ /LfsPTSIzhyzAN5fM+8jSxjfuX/3jxFw6H1H6AmllHX1TiSAdhW82a/x7IHgOer0tQMW 30JVkUy0GSNSpW/TYFYslFAxGyVPEqVpe9a+DHt26feiz1f0vFFLuoKl+KfA/ltEt+ao pLyw== X-Gm-Message-State: AOJu0YzJFO1cVn80gumGYwkG8e2d+wWy66rITlB1pm6UAlbZWBn6RbIP yQ6Y+PDrlGLlfPpVVu2N3vyHTJQ0+cq/5hh+bvsdsTz8bdGBoTek0uX87es+miRVgp3RXBJto5I WkGCuQn1NIbNydwXqPxjup204Oe/n3N9u X-Google-Smtp-Source: AGHT+IGufHK45dl3UzJL3NhpR3mCqL7FaLwLgklkc8xJiQkMET7d/25SFKvuidVDkEOIK8hxrEYAcCfwhwvjb0I+wQo= X-Received: by 2002:a05:6402:2747:b0:57c:d237:4fd with SMTP id 4fb4d7f45d1cf-594ba98f7d9mr4466248a12.4.1720625290634; Wed, 10 Jul 2024 08:28:10 -0700 (PDT) MIME-Version: 1.0 References: <20240709113711.1836747-1-michael@niedermayer.cc> <172053104893.21847.7749073807798840029@lain.khirnov.net> <20240709214329.GD4991@pb2> In-Reply-To: <20240709214329.GD4991@pb2> From: Kacper Michajlow Date: Wed, 10 Jul 2024 17:27:56 +0200 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH v2] avfilter/vf_scale: Cleanup some checks X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Tue, 9 Jul 2024 at 23:43, Michael Niedermayer wrote: > > On Tue, Jul 09, 2024 at 04:46:59PM +0200, Kacper Michajlow wrote: > > On Tue, 9 Jul 2024 at 15:17, Anton Khirnov wrote: > > > > > > Quoting Michael Niedermayer (2024-07-09 13:37:11) > > > > Fixes: CID1513722 Operands don't affect result > > > > > > > > Sponsored-by: Sovereign Tech Fund > > > > Signed-off-by: Michael Niedermayer > > > > --- > > > > libavfilter/vf_scale.c | 6 ++---- > > > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > > > > index bf09196e10d..18e9393d6c1 100644 > > > > --- a/libavfilter/vf_scale.c > > > > +++ b/libavfilter/vf_scale.c > > > > @@ -645,10 +645,8 @@ static int config_props(AVFilterLink *outlink) > > > > if (ret < 0) > > > > goto fail; > > > > > > > > - if (outlink->w > INT_MAX || > > > > - outlink->h > INT_MAX || > > > > - (outlink->h * inlink->w) > INT_MAX || > > > > - (outlink->w * inlink->h) > INT_MAX) > > > > + if ((outlink->h * (int64_t)inlink->w) > INT32_MAX || > > > > + (outlink->w * (int64_t)inlink->h) > INT32_MAX) > > > > > > This does not seems cleaner to me. > > > > > > Also, this check overall seems fishy. Why is it here at all and not e.g. > > > in ff_scale_adjust_dimensions()? Why does it not call > > > av_image_check_size()? Why does it only print a warning and not do > > > anything else? > > I intend to post a better version, but iam a little overworked ATM > so not sure if someone else will do it first. > > > > > > I agree with Anton here. The checks in vf_scale are iffy at best. > > > For starters, this is `outlink->w > INT_MAX` dead check. As the `w` is > > int already. > > that was removed by my patch for that reason. The issue btw originated > by code factorization that replaced int64 by int IIRC > > > > And there is already an UB in `scale_eval_dimensions()` > > which converts double value to int without any checks. > > i try to work on one issue at a time ... > > > > > > I think something like this would make sense to reject big numbers, > > and ofcourse ff_scale_adjust_dimensions() should be more clever about > > overflows too. There is also an overflow in swscale, but that's > > another story. > > > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > > index a1a322ed9e..9483db7564 100644 > > --- a/libavfilter/vf_scale.c > > +++ b/libavfilter/vf_scale.c > > @@ -537,7 +537,7 @@ static int scale_eval_dimensions(AVFilterContext *ctx) > > const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format); > > const AVPixFmtDescriptor *out_desc = av_pix_fmt_desc_get(outlink->format); > > char *expr; > > - int eval_w, eval_h; > > + double eval_w, eval_h; > > int ret; > > double res; > > const AVPixFmtDescriptor *main_desc; > > not a valid patch, that wont apply, its also too messed up formating wise > to review Ah, sorry, I wanted to just point at the issue, with quick diff, but my client destroyed it. I've sent a properly formatted patch now, as I feel it is a good change to do. - Kacper _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".