1) Remove buffer overflow when the number of Driver Binding Protocols increases in...

2) Remove possibility of getting a CR() macro ASSERT() when DisconnectController() is called during a recursive ConnectController()
3) Make sure the DeviceHandle field of the Loaded Image Protocol is always correct
4) Update Loaded Image Protocol logic to guarantee that the DeviceHandle and FilePath fields are correct the image is loaded from a buffer

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@3853 6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
index e8a400c75745ff616c11b06acdc5ae4e155a8417..511acff6bda04bad9f678641a476a6767f13b2b7 100644 (file)
--- a/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
+++ b/MdeModulePkg/Core/Dxe/Hand/DriverSupport.c
@@ -23,20 +23,17 @@ Revision History
 \r
 #include <DxeMain.h>\r
 \r
+BOOLEAN mRepairLoadedImage = FALSE;\r
 \r
-\r
-STATIC\r
+//\r
+// Driver Support Function Prototypes\r
+//\r
 EFI_STATUS\r
 GetHandleFromDriverBinding (\r
   IN EFI_DRIVER_BINDING_PROTOCOL           *DriverBindingNeed,\r
   OUT  EFI_HANDLE                          *Handle \r
   );\r
 \r
-\r
-//\r
-// Driver Support Function Prototypes\r
-//\r
-STATIC\r
 EFI_STATUS \r
 CoreConnectSingleController (\r
   IN  EFI_HANDLE                ControllerHandle,\r
@@ -47,8 +44,6 @@ CoreConnectSingleController (
 //\r
 // Driver Support Functions\r
 //\r
-\r
-\r
 EFI_STATUS \r
 EFIAPI\r
 CoreConnectController (\r
@@ -88,6 +83,14 @@ Returns:
   LIST_ENTRY                           *ProtLink;\r
   OPEN_PROTOCOL_DATA                   *OpenData;\r
   EFI_DEVICE_PATH_PROTOCOL             *AlignedRemainingDevicePath;\r
+  EFI_HANDLE                           *ChildHandleBuffer;\r
+  UINTN                                ChildHandleCount;\r
+  UINTN                                Index;\r
+  EFI_HANDLE                           *LoadedImageHandleBuffer;\r
+  UINTN                                LoadedImageHandleCount;\r
+  LOADED_IMAGE_PRIVATE_DATA            *Image;\r
+  EFI_HANDLE                           DeviceHandle;\r
+  EFI_DEVICE_PATH_PROTOCOL             *DevicePath;\r
   \r
   //\r
   // Make sure ControllerHandle is valid\r
@@ -100,56 +103,179 @@ Returns:
   Handle = ControllerHandle;\r
 \r
   //\r
-  // Connect all drivers to ControllerHandle \r
+  // Make a copy of RemainingDevicePath to guanatee it is aligned\r
   //\r
   AlignedRemainingDevicePath = NULL;\r
   if (RemainingDevicePath != NULL) {\r
     AlignedRemainingDevicePath = CoreDuplicateDevicePath (RemainingDevicePath);\r
   }\r
-  ReturnStatus = CoreConnectSingleController (\r
-                   ControllerHandle,\r
-                   DriverImageHandle,\r
-                   AlignedRemainingDevicePath\r
-                   );\r
-  if (AlignedRemainingDevicePath != NULL) {\r
-    CoreFreePool (AlignedRemainingDevicePath);\r
-  }\r
 \r
   //\r
-  // If not recursive, then just return after connecting drivers to ControllerHandle\r
+  // Connect all drivers to ControllerHandle\r
+  // If CoreConnectSingleController returns EFI_NOT_READY, then the number of\r
+  // Driver Binding Protocols in the handle database has increased during the call\r
+  // so the connect operation must be restarted\r
   //\r
-  if (!Recursive) {\r
-    return ReturnStatus;\r
+  do {\r
+    ReturnStatus = CoreConnectSingleController (\r
+                    ControllerHandle,\r
+                    DriverImageHandle,\r
+                    AlignedRemainingDevicePath\r
+                    );\r
+  } while (ReturnStatus == EFI_NOT_READY);\r
+\r
+  //\r
+  // Free the aligned copy of RemainingDevicePath\r
+  //\r
+  if (AlignedRemainingDevicePath != NULL) {\r
+    CoreFreePool (AlignedRemainingDevicePath);\r
   }\r
 \r
   //\r
   // If recursive, then connect all drivers to all of ControllerHandle's children\r
   //\r
-  CoreAcquireProtocolLock ();\r
-  for (Link = Handle->Protocols.ForwardLink; Link != &Handle->Protocols; Link = Link->ForwardLink) {\r
-    Prot = CR(Link, PROTOCOL_INTERFACE, Link, PROTOCOL_INTERFACE_SIGNATURE);\r
-    for (ProtLink = Prot->OpenList.ForwardLink; \r
-           ProtLink != &Prot->OpenList; \r
-           ProtLink = ProtLink->ForwardLink) {\r
+  if (Recursive) {\r
+    //\r
+    // Acquire the protocol lock on the handle database so the child handles can be collected\r
+    //\r
+    CoreAcquireProtocolLock ();\r
+\r
+    //\r
+    // Make sure the DriverBindingHandle is valid\r
+    //\r
+    Status = CoreValidateHandle (ControllerHandle);\r
+    if (EFI_ERROR (Status)) {\r
+      //\r
+      // Release the protocol lock on the handle database\r
+      //\r
+      CoreReleaseProtocolLock ();\r
+\r
+      return ReturnStatus;\r
+    }\r
+\r
+\r
+    //\r
+    // Count ControllerHandle's children\r
+    //\r
+    for (Link = Handle->Protocols.ForwardLink, ChildHandleCount = 0; Link != &Handle->Protocols; Link = Link->ForwardLink) {\r
+      Prot = CR(Link, PROTOCOL_INTERFACE, Link, PROTOCOL_INTERFACE_SIGNATURE);\r
+      for (ProtLink = Prot->OpenList.ForwardLink; \r
+          ProtLink != &Prot->OpenList; \r
+          ProtLink = ProtLink->ForwardLink) {\r
         OpenData = CR (ProtLink, OPEN_PROTOCOL_DATA, Link, OPEN_PROTOCOL_DATA_SIGNATURE);\r
         if ((OpenData->Attributes & EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) != 0) {\r
-          CoreReleaseProtocolLock ();\r
-          Status = CoreConnectController (\r
-                          OpenData->ControllerHandle,\r
-                          NULL,\r
-                          NULL,\r
-                          TRUE\r
-                          ); \r
-          CoreAcquireProtocolLock ();\r
+          ChildHandleCount++;\r
         }\r
+      }\r
     }\r
+\r
+    //\r
+    // Allocate a handle buffer for ControllerHandle's children\r
+    //\r
+    ChildHandleBuffer = CoreAllocateBootServicesPool (ChildHandleCount * sizeof(EFI_HANDLE));\r
+\r
+    //\r
+    // Fill in a handle buffer with ControllerHandle's children\r
+    //\r
+    for (Link = Handle->Protocols.ForwardLink, ChildHandleCount = 0; Link != &Handle->Protocols; Link = Link->ForwardLink) {\r
+      Prot = CR(Link, PROTOCOL_INTERFACE, Link, PROTOCOL_INTERFACE_SIGNATURE);\r
+      for (ProtLink = Prot->OpenList.ForwardLink; \r
+          ProtLink != &Prot->OpenList; \r
+          ProtLink = ProtLink->ForwardLink) {\r
+        OpenData = CR (ProtLink, OPEN_PROTOCOL_DATA, Link, OPEN_PROTOCOL_DATA_SIGNATURE);\r
+        if ((OpenData->Attributes & EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) != 0) {\r
+          ChildHandleBuffer[ChildHandleCount] = OpenData->ControllerHandle;\r
+          ChildHandleCount++;\r
+        }\r
+      }\r
+    }\r
+\r
+    //\r
+    // Release the protocol lock on the handle database\r
+    //\r
+    CoreReleaseProtocolLock ();\r
+\r
+    //\r
+    // Recursively connect each child handle\r
+    //\r
+    for (Index = 0; Index < ChildHandleCount; Index++) {\r
+      CoreConnectController (\r
+        ChildHandleBuffer[Index],\r
+        NULL,\r
+        NULL,\r
+        TRUE\r
+        ); \r
+    }\r
+\r
+    //\r
+    // Free the handle buffer of ControllerHandle's children\r
+    //\r
+    CoreFreePool (ChildHandleBuffer);\r
   }\r
-  CoreReleaseProtocolLock ();\r
-  \r
+\r
+  //\r
+  // If a Stop() function has been called one or more time successfully, then attempt to \r
+  // repair the stale DeviceHandle fields of the Loaded Image Protocols\r
+  //\r
+  if (mRepairLoadedImage) {\r
+    //\r
+    // Assume that all Loaded Image Protocols can be repaired\r
+    //\r
+    mRepairLoadedImage = FALSE;\r
+\r
+    //\r
+    // Get list of all Loaded Image Protocol Instances\r
+    //\r
+    Status = CoreLocateHandleBuffer (\r
+              ByProtocol,   \r
+              &gEfiLoadedImageProtocolGuid,  \r
+              NULL,\r
+              &LoadedImageHandleCount, \r
+              &LoadedImageHandleBuffer\r
+              );\r
+    if (!EFI_ERROR (Status) && LoadedImageHandleCount != 0) {\r
+      for (Index = 0; Index < LoadedImageHandleCount; Index++) {\r
+        //\r
+        // Retrieve the Loaded Image Protocol\r
+        //\r
+        Image = CoreLoadedImageInfo (LoadedImageHandleBuffer[Index]);\r
+        if (Image != NULL) {\r
+          //\r
+          // Check to see if the DeviceHandle field is a valid handle\r
+          //\r
+          Status = CoreValidateHandle (Image->Info.DeviceHandle);\r
+          if (EFI_ERROR (Status)) {\r
+            //\r
+            // The DeviceHandle field is not valid.\r
+            // Attempt to locate a device handle with a device path that matches the one\r
+            // that was used to originally load the image\r
+            //\r
+            DevicePath = Image->DeviceHandleDevicePath;\r
+            if (DevicePath != NULL) {\r
+              Status = CoreLocateDevicePath (&gEfiDevicePathProtocolGuid, &DevicePath, &DeviceHandle);\r
+              if (!EFI_ERROR (Status) && (DeviceHandle != NULL_HANDLE) && IsDevicePathEnd(DevicePath)) {\r
+                //\r
+                // A device handle with a matching device path was found, so update the Loaded Image Protocol\r
+                // with the device handle discovered\r
+                //\r
+                Image->Info.DeviceHandle = DeviceHandle;\r
+              } else {\r
+                //\r
+                // There is still at least one Loaded Image Protocol that requires repair\r
+                //\r
+                mRepairLoadedImage = TRUE;\r
+              }\r
+            }\r
+          }\r
+        }\r
+      }\r
+      CoreFreePool (LoadedImageHandleBuffer);\r
+    }\r
+  }\r
+\r
   return ReturnStatus;\r
 }\r
 \r
-STATIC\r
 VOID\r
 AddSortedDriverBindingProtocol (\r
   IN      EFI_HANDLE                   DriverBindingHandle,\r
@@ -213,7 +339,7 @@ Returns:
   //\r
   // See if DriverBinding is already in the sorted list\r
   //\r
-  for (Index = 0; Index < *NumberOfSortedDriverBindingProtocols; Index++) {\r
+  for (Index = 0; Index < *NumberOfSortedDriverBindingProtocols && Index < DriverBindingHandleCount; Index++) {\r
     if (DriverBinding == SortedDriverBindingProtocols[Index]) {\r
       return;\r
     }\r
@@ -222,7 +348,9 @@ Returns:
   //\r
   // Add DriverBinding to the end of the list\r
   //\r
-  SortedDriverBindingProtocols[*NumberOfSortedDriverBindingProtocols] = DriverBinding;\r
+  if (*NumberOfSortedDriverBindingProtocols < DriverBindingHandleCount) {\r
+    SortedDriverBindingProtocols[*NumberOfSortedDriverBindingProtocols] = DriverBinding;\r
+  }\r
   *NumberOfSortedDriverBindingProtocols = *NumberOfSortedDriverBindingProtocols + 1;\r
 \r
   //\r
@@ -235,7 +363,6 @@ Returns:
   }\r
 }\r
  \r
-STATIC\r
 EFI_STATUS \r
 CoreConnectSingleController (\r
   IN  EFI_HANDLE                ControllerHandle,\r
@@ -404,6 +531,19 @@ Returns:
   //\r
   CoreFreePool (DriverBindingHandleBuffer);\r
 \r
+  //\r
+  // If the number of Driver Binding Protocols has increased since this function started, then return\r
+  // EFI_NOT_READY, so it will be restarted\r
+  //\r
+  if (NumberOfSortedDriverBindingProtocols > DriverBindingHandleCount) {\r
+    //\r
+    // Free any buffers that were allocated with AllocatePool()\r
+    //\r
+    CoreFreePool (SortedDriverBindingProtocols);\r
+\r
+    return EFI_NOT_READY;\r
+  }\r
+\r
   //\r
   // Sort the remaining DriverBinding Protocol based on their Version field from\r
   // highest to lowest.\r
@@ -555,6 +695,9 @@ Returns:
   OPEN_PROTOCOL_DATA                  *OpenData;\r
   PROTOCOL_INTERFACE                  *Prot;\r
   EFI_DRIVER_BINDING_PROTOCOL         *DriverBinding;\r
+  EFI_HANDLE                          *LoadedImageHandleBuffer;\r
+  UINTN                               LoadedImageHandleCount;\r
+  LOADED_IMAGE_PRIVATE_DATA           *Image;\r
 \r
   //\r
   // Make sure ControllerHandle is valid\r
@@ -756,6 +899,49 @@ Returns:
   }\r
 \r
   if (StopCount > 0) {\r
+    //\r
+    // If the Loaded Image Protocols do not already need to be repaired, then\r
+    // check the status of the DeviceHandle field of all Loaded Image Protocols\r
+    // to determine if any of them now need repair because a sucessful Stop()\r
+    // may have destroyed the DeviceHandle value in the Loaded Image Protocol\r
+    //\r
+    if (!mRepairLoadedImage) {\r
+      //\r
+      // Get list of all Loaded Image Protocol Instances\r
+      //\r
+      Status = CoreLocateHandleBuffer (\r
+                ByProtocol,   \r
+                &gEfiLoadedImageProtocolGuid,  \r
+                NULL,\r
+                &LoadedImageHandleCount, \r
+                &LoadedImageHandleBuffer\r
+                );\r
+      if (!EFI_ERROR (Status) && LoadedImageHandleCount != 0) {\r
+        for (Index = 0; Index < LoadedImageHandleCount; Index++) {\r
+          //\r
+          // Retrieve the Loaded Image Protocol\r
+          //\r
+          Image = CoreLoadedImageInfo (LoadedImageHandleBuffer[Index]);\r
+          if (Image != NULL) {\r
+            //\r
+            // Check to see if the DeviceHandle field is a valid handle\r
+            //\r
+            Status = CoreValidateHandle (Image->Info.DeviceHandle);\r
+            if (EFI_ERROR (Status)) {\r
+              //\r
+              // The DeviceHandle field is not longer a valid handle.  This means\r
+              // that future calls to ConnectController() need to attemp to repair\r
+              // the Loaded Image Protocols with invalid DeviceHandle fields.  Set \r
+              // the flag used by ConnectController().\r
+              //\r
+              mRepairLoadedImage = TRUE;\r
+              break;\r
+            }\r
+          }\r
+        }\r
+        CoreFreePool (LoadedImageHandleBuffer);\r
+      }\r
+    }\r
     Status = EFI_SUCCESS;\r
   } else {\r
     Status = EFI_NOT_FOUND;\r
@@ -770,9 +956,6 @@ Done:
   return Status;\r
 }\r
 \r
-\r
-\r
-STATIC\r
 EFI_STATUS\r
 GetHandleFromDriverBinding (\r
   IN   EFI_DRIVER_BINDING_PROTOCOL           *DriverBindingNeed,\r

diff --git a/MdeModulePkg/Core/Dxe/Image.h b/MdeModulePkg/Core/Dxe/Image.h
index 87214d35d532e61c8d9d1b1102d7d4e7b8b4856a..8d57de011163d790446ed77240fcae694ece8410 100644 (file)
--- a/MdeModulePkg/Core/Dxe/Image.h
+++ b/MdeModulePkg/Core/Dxe/Image.h
@@ -55,6 +55,8 @@ typedef struct {
 \r
     EFI_RUNTIME_IMAGE_ENTRY     *RuntimeData;   // Runtime image list\r
 \r
+    EFI_DEVICE_PATH_PROTOCOL      *DeviceHandleDevicePath;\r
+\r
     PE_COFF_LOADER_IMAGE_CONTEXT  ImageContext; // PeCoffLoader ImageContext\r
 \r
 } LOADED_IMAGE_PRIVATE_DATA;\r
@@ -98,7 +100,7 @@ CoreOpenImageFile (
   IN BOOLEAN                        BootPolicy,\r
   IN VOID                           *SourceBuffer   OPTIONAL,\r
   IN UINTN                          SourceSize,\r
-  IN OUT EFI_DEVICE_PATH_PROTOCOL   *FilePath,\r
+  IN EFI_DEVICE_PATH_PROTOCOL       *FilePath,\r
   OUT EFI_HANDLE                    *DeviceHandle,\r
   IN IMAGE_FILE_HANDLE              *ImageFileHandle,\r
   OUT UINT32                        *AuthenticationStatus\r

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index daf5fd180b7f815dce19a040757f8d63b65f2fe0..e227f3a6bfc961bd22822221954484fb28cfdae6 100644 (file)
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -74,6 +74,7 @@ LOADED_IMAGE_PRIVATE_DATA mCorePrivateImage  = {
   0,                          // Machine\r
   NULL,                       // Ebc\r
   NULL,                       // RuntimeData\r
+  NULL,                       // DeviceHandleDevicePath\r
 };\r
 \r
 \r
@@ -644,6 +645,7 @@ Returns:
   if (!EFI_ERROR (Status)) {\r
     FilePathSize = CoreDevicePathSize (HandleFilePath) - sizeof(EFI_DEVICE_PATH_PROTOCOL);\r
     FilePath = (EFI_DEVICE_PATH_PROTOCOL *) ( ((UINT8 *)FilePath) + FilePathSize );\r
+    Image->DeviceHandleDevicePath = CoreDuplicateDevicePath (HandleFilePath);\r
   }\r
 \r
   //\r
@@ -656,6 +658,7 @@ Returns:
   Image->Info.FilePath     = CoreDuplicateDevicePath (FilePath);\r
   Image->Info.ParentHandle = ParentImageHandle;\r
 \r
+\r
   if (NumberOfPages != NULL) {\r
     Image->NumberOfPages = *NumberOfPages ;\r
   } else {\r
@@ -1170,6 +1173,10 @@ Returns:
     CoreFreePool (Image->Info.FilePath);\r
   }\r
 \r
+  if (Image->DeviceHandleDevicePath != NULL) {\r
+    CoreFreePool (Image->DeviceHandleDevicePath);\r
+  }\r
+\r
   if (Image->FixupData != NULL) {\r
     CoreFreePool (Image->FixupData);\r
   }\r

diff --git a/MdeModulePkg/Core/Dxe/Image/ImageFile.c b/MdeModulePkg/Core/Dxe/Image/ImageFile.c
index 029f7cf2a7830d2ec62898fd9861e2b6c56c058e..31b1443697f1caba8b1ca466633a1647abba8063 100644 (file)
--- a/MdeModulePkg/Core/Dxe/Image/ImageFile.c
+++ b/MdeModulePkg/Core/Dxe/Image/ImageFile.c
@@ -30,7 +30,7 @@ CoreOpenImageFile (
   IN BOOLEAN                        BootPolicy,\r
   IN VOID                           *SourceBuffer   OPTIONAL,\r
   IN UINTN                          SourceSize,\r
-  IN OUT EFI_DEVICE_PATH_PROTOCOL   *FilePath,\r
+  IN EFI_DEVICE_PATH_PROTOCOL       *FilePath,\r
   OUT EFI_HANDLE                    *DeviceHandle,\r
   IN IMAGE_FILE_HANDLE              *ImageFileHandle,\r
   OUT UINT32                        *AuthenticationStatus\r
@@ -95,6 +95,7 @@ Returns:
     ImageFileHandle->Source     = SourceBuffer;\r
     ImageFileHandle->SourceSize = SourceSize;\r
     *DeviceHandle     = NULL;\r
+    CoreLocateDevicePath (&gEfiDevicePathProtocolGuid, &FilePath, DeviceHandle);\r
     if (SourceSize > 0) {\r
       Status = EFI_SUCCESS;\r
     } else {\r