This code makes me wish for an in-place conversion lib, there really is no reason for dynamic allocation here...
Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>
> On 27. Jan 2023, at 15:27, Pedro Falcato <pedro.falcato@gmail.com> wrote:
>
> On Fri, Jan 27, 2023 at 9:29 AM Savva Mitrofanov <savvamtr@gmail.com> wrote:
>>
>> Missing check in some cases leads to failed StrCpyS call in
>> Ext4GetVolumeLabelInfo. Also correct condition that checks Inode pointer
>> for being NULL in Ext4AllocateInode
>>
>> Cc: Marvin Häuser <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(-)
>>
>> 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 (
>>
>>
>> VolNameLength = StrLen (VolumeName);
>>
>> } else {
>>
>> - VolumeName = AllocateZeroPool (sizeof (CHAR16));
>>
>> + VolumeName = AllocateZeroPool (sizeof (CHAR16));
>>
>> + if (VolumeName == NULL) {
>>
>> + return EFI_OUT_OF_RESOURCES;
>>
>> + }
>>
>> +
>>
>> VolNameLength = 0;
>>
>> }
>>
>>
>>
>> @@ -786,7 +790,9 @@ Ext4GetFilesystemInfo (
>> Info->VolumeSize = MultU64x32 (TotalBlocks, Part->BlockSize);
>>
>> Info->FreeSpace = MultU64x32 (FreeBlocks, Part->BlockSize);
>>
>>
>>
>> - StrCpyS (Info->VolumeLabel, VolNameLength + 1, VolumeName);
>>
>> + Status = StrCpyS (Info->VolumeLabel, VolNameLength + 1, VolumeName);
>>
>> +
>>
>> + ASSERT_EFI_ERROR (Status);
>>
>>
>>
>> FreePool (VolumeName);
>>
>>
>>
>> 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 (
>>
>>
>> Inode = AllocateZeroPool (InodeSize);
>>
>>
>>
>> - if (!Inode) {
>>
>> + if (Inode == NULL) {
>>
>> return NULL;
>>
>> }
>>
>>
>>
>> --
>> 2.39.0
>>
>
> Embarrassing...
> Reviewed-by: Pedro Falcato <pedro.falcato@gmail.com>
>
> --
> Pedro
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99175): https://edk2.groups.io/g/devel/message/99175
Mute This Topic: https://groups.io/mt/96562698/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-