Reviewed-by: Zhichao Gao <zhichao....@intel.com> Thanks, Zhichao
> -----Original Message----- > From: mikub...@linux.microsoft.com <mikub...@linux.microsoft.com> > Sent: Wednesday, November 30, 2022 1:33 AM > To: devel@edk2.groups.io > Cc: Erich McMillan <emcmil...@microsoft.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Michael Kubacki > <mikub...@linux.microsoft.com>; Ni, Ray <ray...@intel.com>; Gao, Zhichao > <zhichao....@intel.com> > Subject: [PATCH v2 09/12] ShellPkg: Fix conditionally uninitialized variables > > From: Michael Kubacki <michael.kuba...@microsoft.com> > > Fixes CodeQL alerts for CWE-457: > https://cwe.mitre.org/data/definitions/457.html > > Cc: Erich McMillan <emcmil...@microsoft.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Michael Kubacki <mikub...@linux.microsoft.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Zhichao Gao <zhichao....@intel.com> > Co-authored-by: Erich McMillan <emcmil...@microsoft.com> > Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com> > --- > ShellPkg/Application/Shell/Shell.c | 1 + > ShellPkg/Application/Shell/ShellProtocol.c | 60 > ++++++++++---------- > ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c | 56 > +++++++++--------- > ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c | 18 +++--- > ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c | 9 ++- > ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c | 14 +++-- > ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c | 17 ++++-- > ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c | 21 +++---- > 8 files changed, 107 insertions(+), 89 deletions(-) > > diff --git a/ShellPkg/Application/Shell/Shell.c > b/ShellPkg/Application/Shell/Shell.c > index df00adfdfa5b..89a677a32978 100644 > --- a/ShellPkg/Application/Shell/Shell.c > +++ b/ShellPkg/Application/Shell/Shell.c > @@ -1300,6 +1300,7 @@ DoStartupScript ( > CHAR16 *FullFileStringPath; > UINTN NewSize; > > + CalleeStatus = EFI_SUCCESS; > Key.UnicodeChar = CHAR_NULL; > Key.ScanCode = 0; > > diff --git a/ShellPkg/Application/Shell/ShellProtocol.c > b/ShellPkg/Application/Shell/ShellProtocol.c > index 509eb60e40f4..d04b47538b96 100644 > --- a/ShellPkg/Application/Shell/ShellProtocol.c > +++ b/ShellPkg/Application/Shell/ShellProtocol.c > @@ -729,50 +729,52 @@ EfiShellGetDeviceName ( > // > // Now check the parent controller using this as the child. > // > - if (DeviceNameToReturn == NULL) { > - PARSE_HANDLE_DATABASE_PARENTS (DeviceHandle, > &ParentControllerCount, &ParentControllerBuffer); > + Status = PARSE_HANDLE_DATABASE_PARENTS (DeviceHandle, > &ParentControllerCount, &ParentControllerBuffer); > + if ((DeviceNameToReturn == NULL) && !EFI_ERROR (Status)) { > for (LoopVar = 0; LoopVar < ParentControllerCount; LoopVar++) { > - PARSE_HANDLE_DATABASE_UEFI_DRIVERS > (ParentControllerBuffer[LoopVar], &ParentDriverCount, > &ParentDriverBuffer); > - for (HandleCount = 0; HandleCount < ParentDriverCount; > HandleCount++) { > - // > - // try using that driver's component name with controller and our > driver as the child. > - // > - Status = gBS->OpenProtocol ( > - ParentDriverBuffer[HandleCount], > - &gEfiComponentName2ProtocolGuid, > - (VOID **)&CompName2, > - gImageHandle, > - NULL, > - EFI_OPEN_PROTOCOL_GET_PROTOCOL > - ); > - if (EFI_ERROR (Status)) { > + Status = PARSE_HANDLE_DATABASE_UEFI_DRIVERS > (ParentControllerBuffer[LoopVar], &ParentDriverCount, > &ParentDriverBuffer); > + if (!EFI_ERROR (Status)) { > + for (HandleCount = 0; HandleCount < ParentDriverCount; > HandleCount++) { > + // > + // try using that driver's component name with controller and our > driver as the child. > + // > Status = gBS->OpenProtocol ( > ParentDriverBuffer[HandleCount], > - &gEfiComponentNameProtocolGuid, > + &gEfiComponentName2ProtocolGuid, > (VOID **)&CompName2, > gImageHandle, > NULL, > EFI_OPEN_PROTOCOL_GET_PROTOCOL > ); > - } > + if (EFI_ERROR (Status)) { > + Status = gBS->OpenProtocol ( > + ParentDriverBuffer[HandleCount], > + &gEfiComponentNameProtocolGuid, > + (VOID **)&CompName2, > + gImageHandle, > + NULL, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL > + ); > + } > + > + if (EFI_ERROR (Status)) { > + continue; > + } > > - if (EFI_ERROR (Status)) { > - continue; > + Lang = GetBestLanguageForDriver (CompName2- > >SupportedLanguages, Language, FALSE); > + Status = CompName2->GetControllerName (CompName2, > ParentControllerBuffer[LoopVar], DeviceHandle, Lang, > &DeviceNameToReturn); > + FreePool (Lang); > + Lang = NULL; > + if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) { > + break; > + } > } > > - Lang = GetBestLanguageForDriver (CompName2- > >SupportedLanguages, Language, FALSE); > - Status = CompName2->GetControllerName (CompName2, > ParentControllerBuffer[LoopVar], DeviceHandle, Lang, > &DeviceNameToReturn); > - FreePool (Lang); > - Lang = NULL; > + SHELL_FREE_NON_NULL (ParentDriverBuffer); > if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) { > break; > } > } > - > - SHELL_FREE_NON_NULL (ParentDriverBuffer); > - if (!EFI_ERROR (Status) && (DeviceNameToReturn != NULL)) { > - break; > - } > } > > SHELL_FREE_NON_NULL (ParentControllerBuffer); diff --git > a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c > b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c > index 36cf46fb2c38..4549cbde9b9a 100644 > --- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c > +++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c > @@ -1399,10 +1399,11 @@ ShellCommandCreateInitialMappingsAndPaths ( > CHAR16 *MapName; > SHELL_MAP_LIST *MapListItem; > > - SplitCurDir = NULL; > - MapName = NULL; > - MapListItem = NULL; > - HandleList = NULL; > + ConsistMappingTable = NULL; > + SplitCurDir = NULL; > + MapName = NULL; > + MapListItem = NULL; > + HandleList = NULL; > > // > // Reset the static members back to zero @@ -1458,32 +1459,35 @@ > ShellCommandCreateInitialMappingsAndPaths ( > // > PerformQuickSort (DevicePathList, Count, sizeof > (EFI_DEVICE_PATH_PROTOCOL *), DevicePathCompare); > > - ShellCommandConsistMappingInitialize (&ConsistMappingTable); > - // > - // Assign new Mappings to all... > - // > - for (Count = 0; HandleList[Count] != NULL; Count++) { > + if (!EFI_ERROR (ShellCommandConsistMappingInitialize > + (&ConsistMappingTable))) { > // > - // Get default name first > + // Assign new Mappings to all... > // > - NewDefaultName = ShellCommandCreateNewMappingName > (MappingTypeFileSystem); > - ASSERT (NewDefaultName != NULL); > - Status = ShellCommandAddMapItemAndUpdatePath (NewDefaultName, > DevicePathList[Count], 0, TRUE); > - ASSERT_EFI_ERROR (Status); > - FreePool (NewDefaultName); > - > - // > - // Now do consistent name > - // > - NewConsistName = ShellCommandConsistMappingGenMappingName > (DevicePathList[Count], ConsistMappingTable); > - if (NewConsistName != NULL) { > - Status = ShellCommandAddMapItemAndUpdatePath > (NewConsistName, DevicePathList[Count], 0, FALSE); > + for (Count = 0; HandleList[Count] != NULL; Count++) { > + // > + // Get default name first > + // > + NewDefaultName = ShellCommandCreateNewMappingName > (MappingTypeFileSystem); > + ASSERT (NewDefaultName != NULL); > + Status = ShellCommandAddMapItemAndUpdatePath > (NewDefaultName, > + DevicePathList[Count], 0, TRUE); > ASSERT_EFI_ERROR (Status); > - FreePool (NewConsistName); > + FreePool (NewDefaultName); > + > + // > + // Now do consistent name > + // > + NewConsistName = ShellCommandConsistMappingGenMappingName > (DevicePathList[Count], ConsistMappingTable); > + if (NewConsistName != NULL) { > + Status = ShellCommandAddMapItemAndUpdatePath > (NewConsistName, DevicePathList[Count], 0, FALSE); > + ASSERT_EFI_ERROR (Status); > + FreePool (NewConsistName); > + } > } > } > > - ShellCommandConsistMappingUnInitialize (ConsistMappingTable); > + if (ConsistMappingTable != NULL) { > + ShellCommandConsistMappingUnInitialize (ConsistMappingTable); > + } > > SHELL_FREE_NON_NULL (HandleList); > SHELL_FREE_NON_NULL (DevicePathList); @@ -1626,12 +1630,12 @@ > ShellCommandUpdateMapping ( > // > PerformQuickSort (DevicePathList, Count, sizeof > (EFI_DEVICE_PATH_PROTOCOL *), DevicePathCompare); > > - ShellCommandConsistMappingInitialize (&ConsistMappingTable); > + Status = ShellCommandConsistMappingInitialize > + (&ConsistMappingTable); > > // > // Assign new Mappings to remainders > // > - for (Count = 0; !EFI_ERROR (Status) && HandleList[Count] != NULL > && !EFI_ERROR (Status); Count++) { > + for (Count = 0; !EFI_ERROR (Status) && HandleList[Count] != NULL; > + Count++) { > // > // Skip ones that already have > // > diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c > index 97a4b57a932f..5329b559ba46 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c > @@ -158,7 +158,10 @@ ShellCommandRunDblk ( > ShellStatus = SHELL_INVALID_PARAMETER; > } > > - ShellConvertStringToUint64 (LbaString, &Lba, TRUE, FALSE); > + if (EFI_ERROR (ShellConvertStringToUint64 (LbaString, &Lba, TRUE, > FALSE))) { > + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), > gShellDebug1HiiHandle, L"dblk", LbaString); > + ShellStatus = SHELL_INVALID_PARAMETER; > + } > } > > if (BlockCountString == NULL) { > @@ -169,12 +172,13 @@ ShellCommandRunDblk ( > ShellStatus = SHELL_INVALID_PARAMETER; > } > > - ShellConvertStringToUint64 (BlockCountString, &BlockCount, TRUE, > FALSE); > - if (BlockCount > 0x10) { > - BlockCount = 0x10; > - } else if (BlockCount == 0) { > - ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), > gShellDebug1HiiHandle, L"dblk", BlockCountString); > - ShellStatus = SHELL_INVALID_PARAMETER; > + if (!EFI_ERROR (ShellConvertStringToUint64 (BlockCountString, > &BlockCount, TRUE, FALSE))) { > + if (BlockCount > 0x10) { > + BlockCount = 0x10; > + } else if (BlockCount == 0) { > + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), > gShellDebug1HiiHandle, L"dblk", BlockCountString); > + ShellStatus = SHELL_INVALID_PARAMETER; > + } > } > } > > diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c > index 8bf23a2076a1..72f8c087cb69 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/EfiDecompress.c > @@ -112,10 +112,13 @@ ShellCommandRunEfiDecompress ( > > if (ShellStatus == SHELL_SUCCESS) { > Status = FileHandleGetSize (InFileHandle, &Temp64Bit); > - ASSERT (Temp64Bit <= (UINT32)(-1)); > - InSize = (UINTN)Temp64Bit; > ASSERT_EFI_ERROR (Status); > - InBuffer = AllocateZeroPool (InSize); > + if (!EFI_ERROR (Status)) { > + ASSERT (Temp64Bit <= (UINT32)(-1)); > + InSize = (UINTN)Temp64Bit; > + InBuffer = AllocateZeroPool (InSize); > + } > + > if (InBuffer == NULL) { > Status = EFI_OUT_OF_RESOURCES; > } else { > diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c > b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c > index d7a133c0c5b4..870c5b0d1da7 100644 > --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c > +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c > @@ -508,9 +508,10 @@ ShellCommandRunConnect ( > Count = ShellCommandLineGetCount (Package); > > if (Param1 != NULL) { > - Status = ShellConvertStringToUint64 (Param1, &Intermediate, TRUE, > FALSE); > - Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate); > - if (EFI_ERROR (Status)) { > + Status = ShellConvertStringToUint64 (Param1, &Intermediate, TRUE, > FALSE); > + if (!EFI_ERROR (Status)) { > + Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate); > + } else { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDLE), > gShellDriver1HiiHandle, L"connect", Param1); > ShellStatus = SHELL_INVALID_PARAMETER; > } > @@ -519,9 +520,10 @@ ShellCommandRunConnect ( > } > > if (Param2 != NULL) { > - Status = ShellConvertStringToUint64 (Param2, &Intermediate, TRUE, > FALSE); > - Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate); > - if (EFI_ERROR (Status)) { > + Status = ShellConvertStringToUint64 (Param2, &Intermediate, TRUE, > FALSE); > + if (!EFI_ERROR (Status)) { > + Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate); > + } else { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDLE), > gShellDriver1HiiHandle, L"connect", Param2); > ShellStatus = SHELL_INVALID_PARAMETER; > } > diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c > b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c > index 009ae5282b27..fd49d1f7ceb4 100644 > --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c > +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Disconnect.c > @@ -160,12 +160,17 @@ ShellCommandRunDisconnect ( > Param1 = ShellCommandLineGetRawValue (Package, 1); > Param2 = ShellCommandLineGetRawValue (Package, 2); > Param3 = ShellCommandLineGetRawValue (Package, 3); > - ShellConvertStringToUint64 (Param1, &Intermediate1, TRUE, FALSE); > - Handle1 = Param1 != NULL ? ConvertHandleIndexToHandle > ((UINTN)Intermediate1) : NULL; > - ShellConvertStringToUint64 (Param2, &Intermediate2, TRUE, FALSE); > - Handle2 = Param2 != NULL ? ConvertHandleIndexToHandle > ((UINTN)Intermediate2) : NULL; > - ShellConvertStringToUint64 (Param3, &Intermediate3, TRUE, FALSE); > - Handle3 = Param3 != NULL ? ConvertHandleIndexToHandle > ((UINTN)Intermediate3) : NULL; > + if (!EFI_ERROR (ShellConvertStringToUint64 (Param1, &Intermediate1, > TRUE, FALSE))) { > + Handle1 = Param1 != NULL ? ConvertHandleIndexToHandle > ((UINTN)Intermediate1) : NULL; > + } > + > + if (!EFI_ERROR (ShellConvertStringToUint64 (Param2, &Intermediate2, > TRUE, FALSE))) { > + Handle2 = Param2 != NULL ? ConvertHandleIndexToHandle > ((UINTN)Intermediate2) : NULL; > + } > + > + if (!EFI_ERROR (ShellConvertStringToUint64 (Param3, &Intermediate3, > TRUE, FALSE))) { > + Handle3 = Param3 != NULL ? ConvertHandleIndexToHandle > ((UINTN)Intermediate3) : NULL; > + } > > if ((Param1 != NULL) && (Handle1 == NULL)) { > ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDLE), > gShellDriver1HiiHandle, L"disconnect", Param1); diff --git > a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c > b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c > index c645c9fd6882..8f70d6b6af39 100644 > --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c > +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/DrvDiag.c > @@ -438,25 +438,22 @@ ShellCommandRunDrvDiag ( > ControllerHandleStr = ShellCommandLineGetRawValue (Package, 2); > ChildHandleStr = ShellCommandLineGetRawValue (Package, 3); > > - if (DriverHandleStr == NULL) { > - Handle1 = NULL; > - } else { > - ShellConvertStringToUint64 (DriverHandleStr, &Intermediate, TRUE, > FALSE); > + if ((DriverHandleStr != NULL) && ShellConvertStringToUint64 > + (DriverHandleStr, &Intermediate, TRUE, FALSE)) { > Handle1 = ConvertHandleIndexToHandle ((UINTN)Intermediate); > + } else { > + Handle1 = NULL; > } > > - if (ControllerHandleStr == NULL) { > - Handle2 = NULL; > - } else { > - ShellConvertStringToUint64 (ControllerHandleStr, &Intermediate, TRUE, > FALSE); > + if ((ControllerHandleStr != NULL) && ShellConvertStringToUint64 > + (ControllerHandleStr, &Intermediate, TRUE, FALSE)) { > Handle2 = ConvertHandleIndexToHandle ((UINTN)Intermediate); > + } else { > + Handle2 = NULL; > } > > - if (ChildHandleStr == NULL) { > - Handle3 = NULL; > - } else { > - ShellConvertStringToUint64 (ChildHandleStr, &Intermediate, TRUE, > FALSE); > + if ((ChildHandleStr != NULL) && ShellConvertStringToUint64 > + (ChildHandleStr, &Intermediate, TRUE, FALSE)) { > Handle3 = ConvertHandleIndexToHandle ((UINTN)Intermediate); > + } else { > + Handle3 = NULL; > } > > Status = DoDiagnostics ( > -- > 2.28.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96726): https://edk2.groups.io/g/devel/message/96726 Mute This Topic: https://groups.io/mt/95339715/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
