From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f49.google.com (mail-ed1-f49.google.com [209.85.208.49]) by mx.groups.io with SMTP id smtpd.web10.106079.1674835814331972757 for ; Fri, 27 Jan 2023 08:10:14 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=S9jzMnoF; spf=pass (domain: gmail.com, ip: 209.85.208.49, mailfrom: savvamtr@gmail.com) Received: by mail-ed1-f49.google.com with SMTP id s3so5175746edd.4 for ; Fri, 27 Jan 2023 08:10:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=tXXY5A5jHBsMGD1bIKBoIRJf8eZEP5iMuQdV0arIrNw=; b=S9jzMnoFwC88exsOg2CC24uIP13u341B1zwzVv++1j/EFoB/1fhSkt3eMoIyIhgdWf awpXC9qli5AfZ9XoSlK0e1ifNWFUYmU9CAH80Rk8eL+EmVHmjVLHP+hg1EMwtiKKJcEr 2uiFNRzOoX2NBHjvVCJQG9qByvaVLV/2+vVHBO7U70wKQUP2UfMOSqV0ziHHNhGym9oW GB9SCsKiSigtJPSlMyrGORqbVFDhPziXOzBtx6BcqBdFh7IBcY8MhqhBIvyd4UsTgNWE Cz5EnP02p68lpYIdo4uoeK/A+Jk4Esb20FvE103fkkXMvVO259n2277z/KhYHbYtvaQB XuHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=tXXY5A5jHBsMGD1bIKBoIRJf8eZEP5iMuQdV0arIrNw=; b=mAqtKnO3zbAKi8wbIA3GzhvHDooWU3Ec1N555NEf1i6MTrikKBk+8E/2q7OinQfUms s3jD1x8D4GTXURqFqsqQ2/hp+vPs3PNpmIH/lrYi9ueIhA8IIaMLaLayakpmPzWsJuAY YYfN27C10Gm8YwpXsT3d1vbOlJR6+JtkHmQYy954r3DLjwmbUcafFK9e8MGzdoo61C5R qHLfXKBgVtQS3hA6i56Z6TtfEibR5uvCTXbt6bpVl2BAD6Vh9xt4kCD9WeTpz8GllpTe U+C6Hg7IxSc9o/0cRXJMRvV8XhyEYqgYmsY91dErhQmy1ealVRa0kRxZLbmNkwLsER7H apBQ== X-Gm-Message-State: AO0yUKUbSMEhgVsm9TvB85fSvG/BbJ3VjENsLaM/V+VvwRPDIWpvub89 GPJzH0EPwJ/oouQD4rO9A/k= X-Google-Smtp-Source: AK7set8mPZiCsBGKfBzzJXheuMawJyXdlCxFltLjkx3Ify5Z1w5ESH2/WQ1pNVgIEQ7tQLnxYtKBQQ== X-Received: by 2002:aa7:d387:0:b0:4a1:9fb0:8760 with SMTP id x7-20020aa7d387000000b004a19fb08760mr4033766edq.36.1674835812821; Fri, 27 Jan 2023 08:10:12 -0800 (PST) Return-Path: Received: from smtpclient.apple ([109.194.121.137]) by smtp.gmail.com with ESMTPSA id eg55-20020a05640228b700b0049f5ab4fa97sm2523873edb.86.2023.01.27.08.10.11 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 27 Jan 2023 08:10:12 -0800 (PST) Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\)) Subject: Re: [edk2-platforms][PATCH v3 06/11] Ext4Pkg: Corrects integer overflow check logic in DiskUtil From: "Savva Mitrofanov" In-Reply-To: Date: Fri, 27 Jan 2023 22:10:07 +0600 Cc: devel@edk2.groups.io, =?utf-8?Q?Marvin_H=C3=A4user?= , =?utf-8?B?0JLQuNGC0LDQu9C40Lkg0K7RgNGM0LXQstC40Ycg0KfQtdC/0YbQvtCy?= Message-Id: <0E32D9D3-6AE8-4542-928E-7DE6F8BEE58A@gmail.com> References: <20230127092945.94389-1-savvamtr@gmail.com> <20230127092945.94389-7-savvamtr@gmail.com> To: Pedro Falcato X-Mailer: Apple Mail (2.3696.120.41.1.1) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > Why this whitespace change? Seems code formatter just removed trailing space. If you want so, I can = drop this change in v4. > On 27 Jan 2023, at 20:24, Pedro Falcato = wrote: >=20 > On Fri, Jan 27, 2023 at 9:29 AM Savva Mitrofanov = wrote: >>=20 >> Corrects multiplication overflow check code >>=20 >> 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(-) >>=20 >> 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/BaseOrderedCollectionRedBlackTreeLib/B= aseOrderedCollectionRedBlackTreeLib.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/Ext4Dxe/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. >>=20 >> - if (Offset > DivU64x32 ((UINT64)-1, Partition->BlockSize)) { >> + if ((NumberBlocks !=3D 0) && (DivU64x64Remainder (Offset, = BlockNumber, NULL) !=3D Partition->BlockSize)) { >> return EFI_INVALID_PARAMETER; >> } >>=20 >> - if (Length > (UINTN)-1/Partition->BlockSize) { >> + if ((NumberBlocks !=3D 0) && (Length / NumberBlocks !=3D = Partition->BlockSize)) { >> return EFI_INVALID_PARAMETER; >> } >>=20 >> @@ -94,12 +94,12 @@ Ext4AllocAndReadBlocks ( >>=20 >> Length =3D NumberBlocks * Partition->BlockSize; >>=20 >> - if (Length > (UINTN)-1/Partition->BlockSize) { >> + // Check for integer overflow >> + if ((NumberBlocks !=3D 0) && (Length / NumberBlocks !=3D = Partition->BlockSize)) { >> return NULL; >> } >>=20 >> Buf =3D AllocatePool (Length); >> - >> if (Buf =3D=3D NULL) { >> return NULL; >> } >> -- >> 2.39.0 >>=20 >=20 > I don't really like open coding this, but it does fix my buggy checks. >=20 > In the interest of not adding a new overengineered safe int library, >=20 > Reviewed-by: Pedro Falcato >=20 >=20 > --=20 > Pedro