MdeModulePkg PartitionDxe: Add Re-entry handling logic for BindingStop

There are scenario when the BindingStop service of PartitionDxe driver be
re-entered.

An example will be ejecting a DVD from a SATA DVDROM and then run
"reconnect -r" under shell. In this specific case, part of the calling
stack will be:

PartitionDriverBindingStop() (PartitionDxe) ->
Stop first child handle (PartitionDxe) ->
ScsiDiskFlushBlocksEx() (ScsiDiskDxe) ->
A media change is detected (ScsiDiskDxe) ->
Reinstall of BlockIO(2) protocols (ScsiDiskDxe) ->
Entering PartitionDriverBindingStop() again (PartitionDxe) ->
Potential risk of referencing already stopped child handle (PartitionDxe)
...

The current code has potential issue of referencing of already stopped
child handle. This commit adds re-entry handling logic to resolve such
issue.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>

diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
index 89cc540210efaaa9ee3d053f466887e7a7e85395..1c53bf0233fdda1a82d4078b4d80af15d9a1158b 100644 (file)
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
@@ -407,6 +407,16 @@ PartitionDriverBindingStop (
   Private = NULL;\r
 \r
   if (NumberOfChildren == 0) {\r
+    //\r
+    // In the case of re-entry of the PartitionDriverBindingStop, the\r
+    // NumberOfChildren may not reflect the actual number of children on the\r
+    // bus driver. Hence, additional check is needed here.\r
+    //\r
+    if (HasChildren (ControllerHandle)) {\r
+      DEBUG((EFI_D_ERROR, "PartitionDriverBindingStop: Still has child.\n"));\r
+      return EFI_DEVICE_ERROR;\r
+    }\r
+\r
     //\r
     // Close the bus driver\r
     //\r
@@ -459,35 +469,57 @@ PartitionDriverBindingStop (
 \r
 \r
     Private = PARTITION_DEVICE_FROM_BLOCK_IO_THIS (BlockIo);\r
+    if (Private->InStop) {\r
+      //\r
+      // If the child handle is going to be stopped again during the re-entry\r
+      // of DriverBindingStop, just do nothing.\r
+      //\r
+      break;\r
+    }\r
+    Private->InStop = TRUE;\r
 \r
-    Status = gBS->CloseProtocol (\r
-                    ControllerHandle,\r
-                    &gEfiDiskIoProtocolGuid,\r
-                    This->DriverBindingHandle,\r
-                    ChildHandleBuffer[Index]\r
-                    );\r
+    BlockIo->FlushBlocks (BlockIo);\r
+\r
+    if (BlockIo2 != NULL) {\r
+      Status = BlockIo2->FlushBlocksEx (BlockIo2, NULL);\r
+      DEBUG((EFI_D_ERROR, "PartitionDriverBindingStop: FlushBlocksEx returned with %r\n", Status));\r
+    } else {\r
+      Status = EFI_SUCCESS;\r
+    }\r
+\r
+    gBS->CloseProtocol (\r
+           ControllerHandle,\r
+           &gEfiDiskIoProtocolGuid,\r
+           This->DriverBindingHandle,\r
+           ChildHandleBuffer[Index]\r
+           );\r
     //\r
     // All Software protocols have be freed from the handle so remove it.\r
     // Remove the BlockIo Protocol if has.\r
     // Remove the BlockIo2 Protocol if has.\r
     //\r
     if (BlockIo2 != NULL) {\r
-      BlockIo->FlushBlocks (BlockIo);\r
-      BlockIo2->FlushBlocksEx (BlockIo2, NULL);\r
-      Status = gBS->UninstallMultipleProtocolInterfaces (\r
-                       ChildHandleBuffer[Index],\r
-                       &gEfiDevicePathProtocolGuid,\r
-                       Private->DevicePath,\r
-                       &gEfiBlockIoProtocolGuid,\r
-                       &Private->BlockIo,\r
-                       &gEfiBlockIo2ProtocolGuid,\r
-                       &Private->BlockIo2,\r
-                       Private->EspGuid,\r
-                       NULL,\r
-                       NULL\r
-                       );\r
+      //\r
+      // Some device drivers might re-install the BlockIO(2) protocols for a\r
+      // media change condition. Therefore, if the FlushBlocksEx returned with\r
+      // EFI_MEDIA_CHANGED, just let the BindingStop fail to avoid potential\r
+      // reference of already stopped child handle.\r
+      //\r
+      if (Status != EFI_MEDIA_CHANGED) {\r
+        Status = gBS->UninstallMultipleProtocolInterfaces (\r
+                         ChildHandleBuffer[Index],\r
+                         &gEfiDevicePathProtocolGuid,\r
+                         Private->DevicePath,\r
+                         &gEfiBlockIoProtocolGuid,\r
+                         &Private->BlockIo,\r
+                         &gEfiBlockIo2ProtocolGuid,\r
+                         &Private->BlockIo2,\r
+                         Private->EspGuid,\r
+                         NULL,\r
+                         NULL\r
+                         );\r
+      }\r
     } else {\r
-      BlockIo->FlushBlocks (BlockIo);\r
       Status = gBS->UninstallMultipleProtocolInterfaces (\r
                        ChildHandleBuffer[Index],\r
                        &gEfiDevicePathProtocolGuid,\r
@@ -501,6 +533,7 @@ PartitionDriverBindingStop (
     }\r
 \r
     if (EFI_ERROR (Status)) {\r
+      Private->InStop = FALSE;\r
       gBS->OpenProtocol (\r
              ControllerHandle,\r
              &gEfiDiskIoProtocolGuid,\r
@@ -516,6 +549,9 @@ PartitionDriverBindingStop (
 \r
     if (EFI_ERROR (Status)) {\r
       AllChildrenStopped = FALSE;\r
+      if (Status == EFI_MEDIA_CHANGED) {\r
+        break;\r
+      }\r
     }\r
   }\r
 \r
@@ -1263,3 +1299,41 @@ InitializePartition (
   return Status;\r
 }\r
 \r
+\r
+/**\r
+  Test to see if there is any child on ControllerHandle.\r
+\r
+  @param[in]  ControllerHandle    Handle of device to test.\r
+\r
+  @retval TRUE                    There are children on the ControllerHandle.\r
+  @retval FALSE                   No child is on the ControllerHandle.\r
+\r
+**/\r
+BOOLEAN\r
+HasChildren (\r
+  IN EFI_HANDLE           ControllerHandle\r
+  )\r
+{\r
+  EFI_OPEN_PROTOCOL_INFORMATION_ENTRY  *OpenInfoBuffer;\r
+  UINTN                                EntryCount;\r
+  EFI_STATUS                           Status;\r
+  UINTN                                Index;\r
+\r
+  Status = gBS->OpenProtocolInformation (\r
+                  ControllerHandle,\r
+                  &gEfiDiskIoProtocolGuid,\r
+                  &OpenInfoBuffer,\r
+                  &EntryCount\r
+                  );\r
+  ASSERT_EFI_ERROR (Status);\r
+\r
+  for (Index = 0; Index < EntryCount; Index++) {\r
+    if ((OpenInfoBuffer[Index].Attributes & EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) != 0) {\r
+      break;\r
+    }\r
+  }\r
+  FreePool (OpenInfoBuffer);\r
+\r
+  return (BOOLEAN) (Index < EntryCount);\r
+}\r
+\r

diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h
index 06470f689f11f23db894cfd38d8ed5dff0ed9dd8..7cb19882cb2df0ab8f8ed18b3c2225175adc6de6 100644 (file)
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h
@@ -61,6 +61,7 @@ typedef struct {
   UINT64                    Start;\r
   UINT64                    End;\r
   UINT32                    BlockSize;\r
+  BOOLEAN                   InStop;\r
 \r
   EFI_GUID                  *EspGuid;\r
 \r
@@ -345,6 +346,20 @@ PartitionInstallChildHandle (
   IN  BOOLEAN                      InstallEspGuid\r
   );\r
 \r
+/**\r
+  Test to see if there is any child on ControllerHandle.\r
+\r
+  @param[in]  ControllerHandle    Handle of device to test.\r
+\r
+  @retval TRUE                    There are children on the ControllerHandle.\r
+  @retval FALSE                   No child is on the ControllerHandle.\r
+\r
+**/\r
+BOOLEAN\r
+HasChildren (\r
+  IN EFI_HANDLE           ControllerHandle\r
+  );\r
+\r
 /**\r
   Install child handles if the Handle supports GPT partition structure.\r
 \r