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 <mhaeu...@posteo.de> > On 27. Jan 2023, at 15:27, Pedro Falcato <pedro.falc...@gmail.com> wrote: > > On Fri, Jan 27, 2023 at 9:29 AM Savva Mitrofanov <savva...@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 <mhaeu...@posteo.de> >> Cc: Pedro Falcato <pedro.falc...@gmail.com> >> Cc: Vitaly Cheptsov <vit9...@protonmail.com> >> Fixes: cfbbae595eec ("Ext4Pkg: Add handling of EFI_FILE_SYSTEM_VOLUME_LABEL >> GetInfo().") >> Signed-off-by: Savva Mitrofanov <savva...@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.falc...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-