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 CA81D4BA15 for ; Tue, 9 Jul 2024 21:43:40 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C098468D81B; Wed, 10 Jul 2024 00:43:37 +0300 (EEST) Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0FE9568D81B for ; Wed, 10 Jul 2024 00:43:31 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 67E7B60002 for ; Tue, 9 Jul 2024 21:43:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1720561410; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=3S2yMavc7q8SWU1a4nu4fE00NWVMIaI9pErjL+P3vyk=; b=Bqy2O5Ig669d79hmMPTZJMCvCLqPvsIcFnEvZznYwuLWdWDaBDf9JRbbv6/EUumEz2FpJH 9iUhGG5bk6ZuxUFTDoHj7dwdjv2nlVK8FB3DNW/4+sass2haIOmE0qB/VAXB4rpWzKpNAh 9Z+DRC9yJReuPZHnEvx3dUcF6Re3Qr31ipOAfmD4GbZeT9lYc/ivhSJltXMJOYJPKcywG1 TKVKR+PYMTUrDVI9C62+rBnsw8LNoJ278FUpNd98WfVs/3aRP05VvfqqhiWFlMqi43qu9+ PIMy/iLibMx3Pg9+Z9yHhiHG769/5rZbwc9vYBBb64ntDV79P5aoiuvAJx7GgQ== Date: Tue, 9 Jul 2024 23:43:29 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240709214329.GD4991@pb2> References: <20240709113711.1836747-1-michael@niedermayer.cc> <172053104893.21847.7749073807798840029@lain.khirnov.net> MIME-Version: 1.0 In-Reply-To: X-GND-Sasl: michael@niedermayer.cc 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: multipart/mixed; boundary="===============5964847067515440920==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============5964847067515440920== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="2W4Na7kn/Mq3HLmY" Content-Disposition: inline --2W4Na7kn/Mq3HLmY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > 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 ... >=20 > 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. >=20 > 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 =3D av_pix_fmt_desc_get(inlink->format); > const AVPixFmtDescriptor *out_desc =3D av_pix_fmt_desc_get(outlink->forma= t); > 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 also we generally want to minimize double/float so any switch from int to d= ouble generally feels "wrong" thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato=20 --2W4Na7kn/Mq3HLmY Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZo2u/gAKCRBhHseHBAsP q/qdAJ4/Bg4VWrGlPpj6MhpDEDqaMH4FzgCfQSyOiicSofRNiT/DCoWgxknHAbI= =CiGu -----END PGP SIGNATURE----- --2W4Na7kn/Mq3HLmY-- --===============5964847067515440920== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============5964847067515440920==--