From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.171]) by mx.groups.io with SMTP id smtpd.web10.103068.1674829481137982942 for ; Fri, 27 Jan 2023 06:24:41 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=BlFlRwvi; spf=pass (domain: gmail.com, ip: 209.85.214.171, mailfrom: pedro.falcato@gmail.com) Received: by mail-pl1-f171.google.com with SMTP id p24so5057636plw.11 for ; Fri, 27 Jan 2023 06:24:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=n3g+egbFoyk3WEMpYl3VDf23yxy2APuazJiFiL5NZwU=; b=BlFlRwviClExBjFZ5/K9cFtatqBXM+WXjQkulvrQsU/a57P4C7OmqNTtzbwBfZznU9 tFc6STCZ51XocSQiuKFkagaJSy2b2vxsJySKo7JM2b6WMwKRfIafizpNG0GLNdFZlS9M j00K1QYdxoHmUVUj/YwEZ1pg08vSBg+N0K3xvMQ/z9vQ72CWh59vQz3J/wAY+hAGv7tY sH795N4rMGNWn2+qLSVtpwPy3wf8mi1QVbF6ON5ZFoCa4y73ODRu3Bptn1RdrOYhGHmP G0YLVv+xpKU2g2z2+WP8xXh2UjH7RKKQpYLnkgumDZ6hRXOmlhsqbORyscKiaPvYFyFT 12xg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc: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=n3g+egbFoyk3WEMpYl3VDf23yxy2APuazJiFiL5NZwU=; b=TnlLmqqwDossKG57BI7+57WsoqsmG1DY0JhG3lo963IGhVMcDxeRm16OCyFwebfWej P4hNiyaRrvutY4OQntlD67O5xl0I0trU227KG4b9IQ4hUGgpcrjtWXnhlruAazuCF5yr dO13WidqgoFnBuBHrMRasnGL6gR3mETVu916H15MsBldswH1Lyht2O4Bqiu42AZDq78y nIY0LjbIGJCjcxigp4hWpvV/CtsUegM+9aOZcaNfTSPsCavWBi8yjemdz1M7YrNJtua4 bcQ4YyPpCVd11VhTBL3rZmwImafWP/hqFQlhsSWj0x4bv/zTeWEI5o5zbarnFDr+oJOG Cqew== X-Gm-Message-State: AFqh2kosGWtcTlcJhcBfDTUgJm9uQzJc2RaGCXTaxegiEZzI8CDhpi5C 3ITn0jRGttHxpf+F7ZKQzXKQrEw/BwAwcJ32v48= X-Google-Smtp-Source: AMrXdXspBpwzIXEKnEtV3ieuq8ztCTHDYqB5PtNsfD/jlKrdOTKzTzddjE/gVq/mjX5LQR2lC9ULXeT83QMYJk8vN9k= X-Received: by 2002:a17:902:c3c6:b0:195:e436:6814 with SMTP id j6-20020a170902c3c600b00195e4366814mr3240812plj.14.1674829480684; Fri, 27 Jan 2023 06:24:40 -0800 (PST) MIME-Version: 1.0 References: <20230127092945.94389-1-savvamtr@gmail.com> <20230127092945.94389-7-savvamtr@gmail.com> In-Reply-To: <20230127092945.94389-7-savvamtr@gmail.com> From: "Pedro Falcato" Date: Fri, 27 Jan 2023 14:24:29 +0000 Message-ID: Subject: Re: [edk2-platforms][PATCH v3 06/11] Ext4Pkg: Corrects integer overflow check logic in DiskUtil To: Savva Mitrofanov Cc: devel@edk2.groups.io, =?UTF-8?Q?Marvin_H=C3=A4user?= , Vitaly Cheptsov Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Jan 27, 2023 at 9:29 AM Savva Mitrofanov wrote= : > > Corrects multiplication overflow check code > > Cc: Marvin H=C3=A4user > Cc: Pedro Falcato > Cc: Vitaly Cheptsov > Fixes: d9ceedca6c8f ("Ext4Pkg: Add Ext4Dxe driver.") > Signed-off-by: Savva Mitrofanov > --- > Features/Ext4Pkg/Ext4Pkg.dsc | 2 +- > Features/Ext4Pkg/Ext4Dxe/DiskUtil.c | 8 ++++---- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/Features/Ext4Pkg/Ext4Pkg.dsc b/Features/Ext4Pkg/Ext4Pkg.dsc > index 59bc327ebf6e..621c63eaf92d 100644 > --- a/Features/Ext4Pkg/Ext4Pkg.dsc > +++ b/Features/Ext4Pkg/Ext4Pkg.dsc > @@ -46,7 +46,7 @@ > DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf > OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeL= ib/BaseOrderedCollectionRedBlackTreeLib.inf > BaseUcs2Utf8Lib|RedfishPkg/Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf > - > + Why this whitespace change? > # > # Required for stack protector support > # > diff --git a/Features/Ext4Pkg/Ext4Dxe/DiskUtil.c b/Features/Ext4Pkg/Ext4D= xe/DiskUtil.c > index 32da35f7d9f5..c4af956da926 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/DiskUtil.c > +++ b/Features/Ext4Pkg/Ext4Dxe/DiskUtil.c > @@ -60,11 +60,11 @@ Ext4ReadBlocks ( > // Check for overflow on the block -> byte conversions. > // Partition->BlockSize is never 0, so we don't need to check for that= . > > - if (Offset > DivU64x32 ((UINT64)-1, Partition->BlockSize)) { > + if ((NumberBlocks !=3D 0) && (DivU64x64Remainder (Offset, BlockNumber,= NULL) !=3D Partition->BlockSize)) { > return EFI_INVALID_PARAMETER; > } > > - if (Length > (UINTN)-1/Partition->BlockSize) { > + if ((NumberBlocks !=3D 0) && (Length / NumberBlocks !=3D Partition->Bl= ockSize)) { > return EFI_INVALID_PARAMETER; > } > > @@ -94,12 +94,12 @@ Ext4AllocAndReadBlocks ( > > Length =3D NumberBlocks * Partition->BlockSize; > > - if (Length > (UINTN)-1/Partition->BlockSize) { > + // Check for integer overflow > + if ((NumberBlocks !=3D 0) && (Length / NumberBlocks !=3D Partition->Bl= ockSize)) { > return NULL; > } > > Buf =3D AllocatePool (Length); > - > if (Buf =3D=3D NULL) { > return NULL; > } > -- > 2.39.0 > I don't really like open coding this, but it does fix my buggy checks. In the interest of not adding a new overengineered safe int library, Reviewed-by: Pedro Falcato --=20 Pedro