NetworkPkg: Fix the driver model issue in HTTP Boot driver.

The HTTP Boot driver have some UEFI driver model problems which will make the
code ASSERT when it's disconnected.
First, the driver opens the HttpSb protocol BY_CHILD without BY_DRIVER attribute.
So the driver binding stop won't be called when HTTP driver is disconnected, so
a child handle is left and made HTTP driver binding stop function goes into error.
This patch remove this unnecessary OpenProtocol and only unload the HII from when
both the IP4 and IP6 stack have been stopped completely.
The second issue is the HTTP boot driver always use the driver's image handle as
it's driver binding handle, it's not correct. HTTP Boot driver provides 2 separate
driver binding protocols from IP4 and IP6 stack, so it has 2 driver binding handle.
So this patch fix the code to use correct driver binding handle when create/open
a HTTP child handle.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index aae452729c8ebd4e0c27b7d3bbdc342221158817..0c472938a068145200089aee59ae25db0a80c17c 100644 (file)
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -447,6 +447,7 @@ HttpBootCreateHttpIo (
 {\r
   HTTP_IO_CONFIG_DATA          ConfigData;\r
   EFI_STATUS                   Status;\r
+  EFI_HANDLE                   ImageHandle;\r
 \r
   ASSERT (Private != NULL);\r
 \r
@@ -456,14 +457,16 @@ HttpBootCreateHttpIo (
     ConfigData.Config4.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT;\r
     IP4_COPY_ADDRESS (&ConfigData.Config4.LocalIp, &Private->StationIp.v4);\r
     IP4_COPY_ADDRESS (&ConfigData.Config4.SubnetMask, &Private->SubnetMask.v4);\r
+    ImageHandle = Private->Ip4Nic->ImageHandle;\r
   } else {\r
     ConfigData.Config6.HttpVersion    = HttpVersion11;\r
     ConfigData.Config6.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT;\r
     IP6_COPY_ADDRESS (&ConfigData.Config6.LocalIp, &Private->StationIp.v6);\r
+    ImageHandle = Private->Ip6Nic->ImageHandle;\r
   }\r
 \r
   Status = HttpIoCreateIo (\r
-             Private->Image,\r
+             ImageHandle,\r
              Private->Controller,\r
              Private->UsingIpv6 ? IP_VERSION_6 : IP_VERSION_4,\r
              &ConfigData,\r

diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
index 597192349d6b0c28b89baaaac29727c5c527bc9a..0c1ff43d5c84a856211fbec5cda329a94a450f91 100644 (file)
--- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
@@ -553,7 +553,6 @@ HttpBootConfigFormInit (
   EFI_STATUS                        Status;\r
   HTTP_BOOT_FORM_CALLBACK_INFO      *CallbackInfo;\r
   VENDOR_DEVICE_PATH                VendorDeviceNode;\r
-  EFI_SERVICE_BINDING_PROTOCOL      *HttpSb;\r
   CHAR16                            *MacString;\r
   CHAR16                            *OldMenuString;\r
   CHAR16                            MenuString[128];\r
@@ -600,20 +599,6 @@ HttpBootConfigFormInit (
                   &CallbackInfo->ConfigAccess,\r
                   NULL\r
                   );\r
-  if (!EFI_ERROR (Status)) {\r
-    //\r
-    // Open the Parent Handle for the child\r
-    //\r
-    Status = gBS->OpenProtocol (\r
-                    Private->Controller,\r
-                    &gEfiHttpServiceBindingProtocolGuid,\r
-                    (VOID **) &HttpSb,\r
-                    Private->Image,\r
-                    CallbackInfo->ChildHandle,\r
-                    EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER\r
-                    );\r
-  }\r
-\r
   if (EFI_ERROR (Status)) {\r
     goto Error;\r
   }\r
@@ -636,7 +621,7 @@ HttpBootConfigFormInit (
   //\r
   // Append MAC string in the menu help string\r
   //\r
-  Status = NetLibGetMacString (Private->Controller, Private->Image, &MacString);\r
+  Status = NetLibGetMacString (Private->Controller, NULL, &MacString);\r
   if (!EFI_ERROR (Status)) {\r
     OldMenuString = HiiGetString (\r
                       CallbackInfo->RegisteredHandle, \r
@@ -654,6 +639,7 @@ HttpBootConfigFormInit (
     FreePool (MacString);\r
     FreePool (OldMenuString);\r
 \r
+    CallbackInfo->Initilized = TRUE;\r
     return EFI_SUCCESS;\r
   }\r
   \r
@@ -666,6 +652,7 @@ Error:
 /**\r
   Unload the configuration form, this includes: delete all the configuration\r
   entries, uninstall the form callback protocol, and free the resources used.\r
+  The form will only be unload completely when both IP4 and IP6 stack are stopped.\r
 \r
   @param[in]  Private             Pointer to the driver private data.\r
 \r
@@ -680,18 +667,15 @@ HttpBootConfigFormUnload (
 {\r
   HTTP_BOOT_FORM_CALLBACK_INFO      *CallbackInfo;\r
 \r
-  CallbackInfo = &Private->CallbackInfo;\r
-  if (CallbackInfo->ChildHandle != NULL) {\r
+  if (Private->Ip4Nic != NULL || Private->Ip6Nic != NULL) {\r
     //\r
-    // Close the child handle\r
+    // Only unload the configuration form when both IP4 and IP6 stack are stopped.\r
     //\r
-    gBS->CloseProtocol (\r
-           Private->Controller,\r
-           &gEfiHttpServiceBindingProtocolGuid,\r
-           Private->Image,\r
-           CallbackInfo->ChildHandle\r
-           );\r
-    \r
+    return EFI_SUCCESS;\r
+  }\r
+\r
+  CallbackInfo = &Private->CallbackInfo;\r
+  if (CallbackInfo->ChildHandle != NULL) {\r
     //\r
     // Uninstall EFI_HII_CONFIG_ACCESS_PROTOCOL\r
     //\r

diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.h b/NetworkPkg/HttpBootDxe/HttpBootConfig.h
index a2afd18a88a4401bf65d5a6b3114c58ef4193f2b..e610fe8cd9708c625b44575aaf9a991ac0cac958 100644 (file)
--- a/NetworkPkg/HttpBootDxe/HttpBootConfig.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.h
@@ -63,6 +63,7 @@ HttpBootConfigFormInit (
 /**\r
   Unload the configuration form, this includes: delete all the configuration\r
   entries, uninstall the form callback protocol, and free the resources used.\r
+  The form will only be unload completely when both IP4 and IP6 stack are stopped.\r
 \r
   @param[in]  Private             Pointer to the driver private data.\r
 \r

diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.c b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
index 6a3033db11a0ebeaecdc62c5bdd4903ab5b9a5b5..642e0fe31ee5de9028ed749ab565786c7495ddf1 100644 (file)
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
@@ -321,7 +321,7 @@ HttpBootIp4DxeDriverBindingStart (
                   );\r
 \r
   if (!EFI_ERROR (Status)) {\r
-      Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id);\r
+    Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id);\r
   } else {\r
     //\r
     // Initialize the private data structure.\r
@@ -332,7 +332,6 @@ HttpBootIp4DxeDriverBindingStart (
     }\r
     Private->Signature = HTTP_BOOT_PRIVATE_DATA_SIGNATURE;\r
     Private->Controller = ControllerHandle;\r
-    Private->Image = This->ImageHandle;\r
     InitializeListHead (&Private->CacheList);\r
     //\r
     // Get the NII interface if it exists, it's not required.\r
@@ -399,8 +398,9 @@ HttpBootIp4DxeDriverBindingStart (
   if (Private->Ip4Nic == NULL) {\r
     return EFI_OUT_OF_RESOURCES;\r
   }\r
-  Private->Ip4Nic->Private   = Private;\r
-  Private->Ip4Nic->Signature = HTTP_BOOT_VIRTUAL_NIC_SIGNATURE;\r
+  Private->Ip4Nic->Private     = Private;\r
+  Private->Ip4Nic->ImageHandle = This->DriverBindingHandle;\r
+  Private->Ip4Nic->Signature   = HTTP_BOOT_VIRTUAL_NIC_SIGNATURE;\r
   \r
   //\r
   // Create DHCP4 child instance.\r
@@ -793,7 +793,7 @@ HttpBootIp6DxeDriverBindingStart (
                   );\r
   \r
   if (!EFI_ERROR (Status)) {\r
-      Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id);\r
+    Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id);\r
   } else {\r
     //\r
     // Initialize the private data structure.\r
@@ -804,7 +804,6 @@ HttpBootIp6DxeDriverBindingStart (
     }\r
     Private->Signature = HTTP_BOOT_PRIVATE_DATA_SIGNATURE;\r
     Private->Controller = ControllerHandle;\r
-    Private->Image = This->ImageHandle;\r
     InitializeListHead (&Private->CacheList);\r
     //\r
     // Get the NII interface if it exists, it's not required.\r
@@ -871,9 +870,10 @@ HttpBootIp6DxeDriverBindingStart (
   if (Private->Ip6Nic == NULL) {\r
     return EFI_OUT_OF_RESOURCES;\r
   }\r
-  Private->Ip6Nic->Private   = Private;\r
-  Private->Ip6Nic->Signature = HTTP_BOOT_VIRTUAL_NIC_SIGNATURE;\r
-\r
+  Private->Ip6Nic->Private     = Private;\r
+  Private->Ip6Nic->ImageHandle = This->DriverBindingHandle;\r
+  Private->Ip6Nic->Signature   = HTTP_BOOT_VIRTUAL_NIC_SIGNATURE;\r
+  \r
   //\r
   // Create Dhcp6 child and open Dhcp6 protocol\r
   Status = NetLibCreateServiceChild (\r

diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.h b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
index 7cb4b2c55a14e40a58e9025065cfbcc965321ff5..b3e2576e1e5d079d9a2418707834792fea139ccf 100644 (file)
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
@@ -102,6 +102,7 @@ typedef union {
 struct _HTTP_BOOT_VIRTUAL_NIC {\r
   UINT32                                    Signature;\r
   EFI_HANDLE                                Controller;\r
+  EFI_HANDLE                                ImageHandle;\r
   EFI_LOAD_FILE_PROTOCOL                    LoadFile;\r
   EFI_DEVICE_PATH_PROTOCOL                  *DevicePath;\r
   HTTP_BOOT_PRIVATE_DATA                    *Private;\r
@@ -118,7 +119,6 @@ struct _HTTP_BOOT_VIRTUAL_NIC {
 struct _HTTP_BOOT_PRIVATE_DATA {\r
   UINT32                                    Signature;\r
   EFI_HANDLE                                Controller;\r
-  EFI_HANDLE                                Image;\r
 \r
   HTTP_BOOT_VIRTUAL_NIC                     *Ip4Nic;\r
   HTTP_BOOT_VIRTUAL_NIC                     *Ip6Nic;\r

diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
index 758c49b71286202e4e3949d57e62fe51623a4e5f..bd99a913c6c8cc166d47b9c8b4dc44a6beaa214c 100644 (file)
--- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
@@ -372,7 +372,7 @@ HttpBootDns (
   //\r
   Status = NetLibCreateServiceChild (\r
              Private->Controller,\r
-             Private->Image,\r
+             Private->Ip6Nic->ImageHandle,\r
              &gEfiDns6ServiceBindingProtocolGuid,\r
              &Dns6Handle\r
              );\r
@@ -384,7 +384,7 @@ HttpBootDns (
                   Dns6Handle,\r
                   &gEfiDns6ProtocolGuid,\r
                   (VOID **) &Dns6,\r
-                  Private->Image,\r
+                  Private->Ip6Nic->ImageHandle,\r
                   Private->Controller,\r
                   EFI_OPEN_PROTOCOL_BY_DRIVER\r
                   );\r
@@ -474,7 +474,7 @@ Exit:
     gBS->CloseProtocol (\r
            Dns6Handle,\r
            &gEfiDns6ProtocolGuid,\r
-           Private->Image,\r
+           Private->Ip6Nic->ImageHandle,\r
            Private->Controller\r
            );\r
   }\r
@@ -482,7 +482,7 @@ Exit:
   if (Dns6Handle != NULL) {\r
     NetLibDestroyServiceChild (\r
       Private->Controller,\r
-      Private->Image,\r
+      Private->Ip6Nic->ImageHandle,\r
       &gEfiDns6ServiceBindingProtocolGuid,\r
       Dns6Handle\r
       );\r