From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66])
 by mx.groups.io with SMTP id smtpd.web11.103367.1674830054121519395
 for <devel@edk2.groups.io>;
 Fri, 27 Jan 2023 06:34:14 -0800
Authentication-Results: mx.groups.io;
 dkim=pass header.i=@posteo.de header.s=2017 header.b=gGIzuG9I;
 spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de)
Received: from submission (posteo.de [185.67.36.169]) 
	by mout02.posteo.de (Postfix) with ESMTPS id 229C52406C8
	for <devel@edk2.groups.io>; Fri, 27 Jan 2023 15:34:11 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017;
	t=1674830052; bh=DcaK4uu9p72zJTegdXD+02BJz2N2V7bEI2EqBNRLSVY=;
	h=Subject:From:Date:Cc:To:From;
	b=gGIzuG9I6ywM74qFtF8wuiUPFlLj6XMn54Ul3JkbMpCIa3fvW1nferiN2dWl6rNZP
	 dMF+WPnC90CKsuHoPP3CetTDt63+4icPgaH5SKpNp8HE9EWK8hTc0mpbL2roL31cwP
	 PePJ+t65AfOKmE2O1ExjpTuP3x3w2U8bIbkHu0xIVKHebcYZV7FiadTvobd9sjW1Ha
	 hFtsDRV+hmgI4U2dlK2/3bdC7mW6CGkwWUQmm11fXosOV+KQ+MKmj7hkt+66unjRVf
	 w13BHEwWdYH3DveRs3A3qWK2qF7zYfrt91W9j3E5/YF26Sjxw+rHlAS/KnD7FZzCrY
	 s767ghotiPUnw==
Received: from customer (localhost [127.0.0.1])
	by submission (posteo.de) with ESMTPSA id 4P3KpC318jz9rxH;
	Fri, 27 Jan 2023 15:34:11 +0100 (CET)
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.400.51.1.1\))
Subject: Re: [edk2-devel] [edk2-platforms][PATCH v3 08/11] Ext4Pkg: Check VolumeName allocation correctness in Ext4GetVolumeName
From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= <mhaeuser@posteo.de>
In-Reply-To: <CAKbZUD3tGkg6xH4XnkqEcCbtDe6njtEEKxuXOBo+KT9-=erZpA@mail.gmail.com>
Date: Fri, 27 Jan 2023 14:34:01 +0000
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
 savvamtr@gmail.com,
 Vitaly Cheptsov <vit9696@protonmail.com>
Message-Id: <63E2632E-91DF-4D2E-A07B-E71FA101503A@posteo.de>
References: <20230127092945.94389-1-savvamtr@gmail.com>
 <20230127092945.94389-9-savvamtr@gmail.com>
 <CAKbZUD3tGkg6xH4XnkqEcCbtDe6njtEEKxuXOBo+KT9-=erZpA@mail.gmail.com>
To: Pedro Falcato <pedro.falcato@gmail.com>
Content-Type: text/plain;
	charset=utf-8
Content-Transfer-Encoding: quoted-printable

This code makes me wish for an in-place conversion lib, there really is =
no reason for dynamic allocation here...

Reviewed-by: Marvin H=C3=A4user <mhaeuser@posteo.de>

> On 27. Jan 2023, at 15:27, Pedro Falcato <pedro.falcato@gmail.com> =
wrote:
>=20
> On Fri, Jan 27, 2023 at 9:29 AM Savva Mitrofanov <savvamtr@gmail.com> =
wrote:
>>=20
>> Missing check in some cases leads to failed StrCpyS call in
>> Ext4GetVolumeLabelInfo. Also correct condition that checks Inode =
pointer
>> for being NULL in Ext4AllocateInode
>>=20
>> Cc: Marvin H=C3=A4user <mhaeuser@posteo.de>
>> Cc: Pedro Falcato <pedro.falcato@gmail.com>
>> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
>> Fixes: cfbbae595eec ("Ext4Pkg: Add handling of =
EFI_FILE_SYSTEM_VOLUME_LABEL GetInfo().")
>> Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com>
>> ---
>> Features/Ext4Pkg/Ext4Dxe/File.c  | 10 ++++++++--
>> Features/Ext4Pkg/Ext4Dxe/Inode.c |  2 +-
>> 2 files changed, 9 insertions(+), 3 deletions(-)
>>=20
>> diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c =
b/Features/Ext4Pkg/Ext4Dxe/File.c
>> index 9dde4a5d1a2d..677caf88fbdc 100644
>> --- a/Features/Ext4Pkg/Ext4Dxe/File.c
>> +++ b/Features/Ext4Pkg/Ext4Dxe/File.c
>> @@ -719,7 +719,11 @@ Ext4GetVolumeName (
>>=20
>>=20
>>     VolNameLength =3D StrLen (VolumeName);
>>=20
>>   } else {
>>=20
>> -    VolumeName    =3D AllocateZeroPool (sizeof (CHAR16));
>>=20
>> +    VolumeName =3D AllocateZeroPool (sizeof (CHAR16));
>>=20
>> +    if (VolumeName =3D=3D NULL) {
>>=20
>> +      return EFI_OUT_OF_RESOURCES;
>>=20
>> +    }
>>=20
>> +
>>=20
>>     VolNameLength =3D 0;
>>=20
>>   }
>>=20
>>=20
>>=20
>> @@ -786,7 +790,9 @@ Ext4GetFilesystemInfo (
>>   Info->VolumeSize =3D MultU64x32 (TotalBlocks, Part->BlockSize);
>>=20
>>   Info->FreeSpace  =3D MultU64x32 (FreeBlocks, Part->BlockSize);
>>=20
>>=20
>>=20
>> -  StrCpyS (Info->VolumeLabel, VolNameLength + 1, VolumeName);
>>=20
>> +  Status =3D StrCpyS (Info->VolumeLabel, VolNameLength + 1, =
VolumeName);
>>=20
>> +
>>=20
>> +  ASSERT_EFI_ERROR (Status);
>>=20
>>=20
>>=20
>>   FreePool (VolumeName);
>>=20
>>=20
>>=20
>> diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c =
b/Features/Ext4Pkg/Ext4Dxe/Inode.c
>> index e44b5638599f..90e3eb88f523 100644
>> --- a/Features/Ext4Pkg/Ext4Dxe/Inode.c
>> +++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c
>> @@ -230,7 +230,7 @@ Ext4AllocateInode (
>>=20
>>=20
>>   Inode =3D AllocateZeroPool (InodeSize);
>>=20
>>=20
>>=20
>> -  if (!Inode) {
>>=20
>> +  if (Inode =3D=3D NULL) {
>>=20
>>     return NULL;
>>=20
>>   }
>>=20
>>=20
>>=20
>> --
>> 2.39.0
>>=20
>=20
> Embarrassing...
> Reviewed-by: Pedro Falcato <pedro.falcato@gmail.com>
>=20
> --=20
> Pedro