Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize Handle before using it

["Wang, Sunny (HPS SW)" <sunnywang@hpe.com>]

Looks good to me. 
Reviewed-by: Sunny Wang <sunnywang@hpe.com>

However, I saw the other potential issue below. If you also think the potential issue is valid, you can fix this in the other patch.
We need to consider the case where MDEPKG_NDEBUG is defined. We're using ASSERT_EFI_ERROR to handle the error status from LocateDevicePath() in the following code block, which contains potential bug that we still use uninitialized Handle for the case where LocateDevicePath returns error status. 

Status = gBS->LocateDevicePath (&gEfiBlockIoProtocolGuid, &TempDevicePath, &Handle);  
ASSERT_EFI_ERROR (Status);

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, February 07, 2017 10:54 AM
To: edk2-devel@lists.01.org
Cc: Jeff Fan <jeff.fan@intel.com>
Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Initialize Handle before using it

BmExpandMediaDevicePath contains a bug that it uses the uninitialized Handle.

Since the function is called when the Handle supports BlockIo or SimpleFileSystem, when there is no SimpleFileSystem installed on the Handle, BlockIo should be installed on the Handle.
The fix initializes the Handle by locating the BlockIo protocol from the device path.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 75bd5dc..9f99122 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -991,9 +991,13 @@ BmExpandMediaDevicePath (
     return FileBuffer;
   }
 
+  Status = gBS->LocateDevicePath (&gEfiBlockIoProtocolGuid, 
+ &TempDevicePath, &Handle);  ASSERT_EFI_ERROR (Status);
+
   //
   // For device boot option only pointing to the removable device handle,
-  // should make sure all its children handles (its child partion or media handles) are created and connected. 
+  // should make sure all its children handles (its child partion or 
+ media handles)  // are created and connected.
   //
   gBS->ConnectController (Handle, NULL, NULL, TRUE);
 
@@ -1004,8 +1008,6 @@ BmExpandMediaDevicePath (
   // returned. After the Block IO protocol is reinstalled, subsequent
   // Block IO read/write will success.
   //
-  Status = gBS->LocateDevicePath (&gEfiBlockIoProtocolGuid, &TempDevicePath, &Handle);
-  ASSERT_EFI_ERROR (Status);
   Status = gBS->HandleProtocol (Handle, &gEfiBlockIoProtocolGuid, (VOID **) &BlockIo);
   ASSERT_EFI_ERROR (Status);
   Buffer = AllocatePool (BlockIo->Media->BlockSize);
--
2.9.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel