public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe.
@ 2017-03-15  2:27 Zhang Lubo
  2017-03-16  1:46 ` Fu, Siyuan
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Zhang Lubo @ 2017-03-15  2:27 UTC (permalink / raw)
  To: edk2-devel; +Cc: Wu Jiaxin, Ye Ting, Fu Siyuan

v2: Handle error case in SockCreateChild and fix typo issue

when we destroy the socket Sock and its associated
protocol control block, we need to first close the
parent protocol, then remove the protocol from childHandle
and last to free any data structures that allocated in
CreateChild. But currently, we free the socket data
(Socket ConfigureState) before removing the protocol
form  the childhandle. So if the up layer want to send the
tcp reset packet in it's driver binding stop function, it will failed.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <lubo.zhang@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c  | 10 +--
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c    |  3 +-
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h    |  5 +-
 MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c  | 48 +-----------
 MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h  |  3 +-
 .../Universal/Network/Tcp4Dxe/SockInterface.c      | 88 ++++++++++++++++++++--
 .../Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c     | 12 +--
 7 files changed, 95 insertions(+), 74 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
index 642e453..792db5c 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
@@ -1,9 +1,9 @@
 /** @file
   The driver binding and service binding protocol for IP4 driver.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -920,11 +920,10 @@ Ip4ServiceBindingDestroyChild (
   EFI_STATUS                Status;
   IP4_SERVICE               *IpSb;
   IP4_PROTOCOL              *IpInstance;
   EFI_IP4_PROTOCOL          *Ip4;
   EFI_TPL                   OldTpl;
-  INTN                      State;
 
   if ((This == NULL) || (ChildHandle == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
@@ -958,17 +957,16 @@ Ip4ServiceBindingDestroyChild (
   // A child can be destroyed more than once. For example,
   // Ip4DriverBindingStop will destroy all of its children.
   // when UDP driver is being stopped, it will destroy all
   // the IP child it opens.
   //
-  if (IpInstance->State == IP4_STATE_DESTROY) {
+  if (IpInstance->InDestroy) {
     gBS->RestoreTPL (OldTpl);
     return EFI_SUCCESS;
   }
 
-  State             = IpInstance->State;
-  IpInstance->State = IP4_STATE_DESTROY;
+  IpInstance->InDestroy = TRUE;
 
   //
   // Close the Managed Network protocol.
   //
   gBS->CloseProtocol (
@@ -1007,10 +1005,11 @@ Ip4ServiceBindingDestroyChild (
                   &gEfiIp4ProtocolGuid,
                   &IpInstance->Ip4Proto
                   );
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
   if (EFI_ERROR (Status)) {
+    IpInstance->InDestroy = FALSE;
     goto ON_ERROR;
   }
 
   Status = Ip4CleanProtocol (IpInstance);
   if (EFI_ERROR (Status)) {
@@ -1031,10 +1030,9 @@ Ip4ServiceBindingDestroyChild (
 
   FreePool (IpInstance);
   return EFI_SUCCESS;
 
 ON_ERROR:
-  IpInstance->State = State;
   gBS->RestoreTPL (OldTpl);
 
   return Status;
 }
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
index 91f1a67..23690ff 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
@@ -1,8 +1,8 @@
 /** @file
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -548,10 +548,11 @@ Ip4InitProtocol (
   ZeroMem (IpInstance, sizeof (IP4_PROTOCOL));
 
   IpInstance->Signature = IP4_PROTOCOL_SIGNATURE;
   CopyMem (&IpInstance->Ip4Proto, &mEfiIp4ProtocolTemplete, sizeof (IpInstance->Ip4Proto));
   IpInstance->State     = IP4_STATE_UNCONFIGED;
+  IpInstance->InDestroy   = FALSE;
   IpInstance->Service   = IpSb;
 
   InitializeListHead (&IpInstance->Link);
   NetMapInit  (&IpInstance->RxTokens);
   NetMapInit  (&IpInstance->TxTokens);
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
index 7a7ad9d..eee38b7 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
@@ -1,9 +1,9 @@
 /** @file
   Ip4 internal functions and type defintions.
   
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -67,11 +67,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 // is called, it becomes UNCONFIGED again. If (partly) destroyed, it
 // becomes DESTROY.
 //
 #define IP4_STATE_UNCONFIGED    0
 #define IP4_STATE_CONFIGED      1
-#define IP4_STATE_DESTROY       2
 
 //
 // The state of IP4 service. It starts from UNSTARTED. It transits
 // to STARTED if autoconfigure is started. If default address is
 // configured, it becomes CONFIGED. and if partly destroyed, it goes
@@ -134,10 +133,12 @@ struct _IP4_PROTOCOL {
 
   EFI_IP4_PROTOCOL          Ip4Proto;
   EFI_HANDLE                Handle;
   INTN                      State;
 
+  BOOLEAN                   InDestroy;                      
+
   IP4_SERVICE               *Service;
   LIST_ENTRY                Link;       // Link to all the IP protocol from the service
 
   //
   // User's transmit/receive tokens, and received/deliverd packets
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
index fc78273..0476077 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of the Socket.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -715,20 +715,12 @@ OnError:
 VOID
 SockDestroy (
   IN OUT SOCKET *Sock
   )
 {
-  VOID        *SockProtocol;
-  EFI_GUID    *ProtocolGuid;
-  EFI_STATUS  Status;
-
   ASSERT (SockStream == Sock->Type);
 
-  if (Sock->DestroyCallback != NULL) {
-    Sock->DestroyCallback (Sock, Sock->Context);
-  }
-
   //
   // Flush the completion token buffered
   // by sock and rcv, snd buffer
   //
   if (!SOCK_IS_UNCONFIGURED (Sock)) {
@@ -760,48 +752,10 @@ SockDestroy (
       );
 
     Sock->Parent = NULL;
   }
 
-  //
-  // Set the protocol guid and driver binding handle
-  // in the light of Sock->SockType
-  //
-  ProtocolGuid = &gEfiTcp4ProtocolGuid;
-
-  //
-  // Retrieve the protocol installed on this sock
-  //
-  Status = gBS->OpenProtocol (
-                  Sock->SockHandle,
-                  ProtocolGuid,
-                  &SockProtocol,
-                  Sock->DriverBinding,
-                  Sock->SockHandle,
-                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
-                  );
-
-  if (EFI_ERROR (Status)) {
-
-    DEBUG ((EFI_D_ERROR, "SockDestroy: Open protocol installed "
-      "on socket failed with %r\n", Status));
-
-    goto FreeSock;
-  }
-
-  //
-  // Uninstall the protocol installed on this sock
-  // in the light of Sock->SockType
-  //
-  gBS->UninstallMultipleProtocolInterfaces (
-        Sock->SockHandle,
-        ProtocolGuid,
-        SockProtocol,
-        NULL
-        );
-
-FreeSock:
   FreePool (Sock);
   return ;
 }
 
 
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
index 4a5a845..d4104a0 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
@@ -1,9 +1,9 @@
 /** @file
   Socket implementation header file.
 
-Copyright (c) 2005 - 2006, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2007, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -14,10 +14,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #ifndef _SOCK_IMPL_H_
 #define _SOCK_IMPL_H_
 
 #include "Socket.h"
+#include "Tcp4Main.h"
 
 /**
   Signal a event with the given status.
   
   @param Token        The token's event is to be signaled.
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
index a8bdef6..f8b535c 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
@@ -1,9 +1,9 @@
 /** @file
   Interface function of the Socket.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -142,30 +142,81 @@ SockBufferToken (
 EFI_STATUS
 SockDestroyChild (
   IN SOCKET *Sock
   )
 {
-  EFI_STATUS  Status;
+  EFI_STATUS       Status;
+  TCP4_PROTO_DATA  *ProtoData;
+  TCP_CB           *Tcb;
+  VOID             *SockProtocol;
 
   ASSERT ((Sock != NULL) && (Sock->ProtoHandler != NULL));
 
   if (Sock->InDestroy) {
     return EFI_SUCCESS;
   }
 
   Sock->InDestroy = TRUE;
 
+  ProtoData = (TCP4_PROTO_DATA *) Sock->ProtoReserved;
+  Tcb       = ProtoData->TcpPcb;
+
+  ASSERT (Tcb != NULL);
+
   Status = EfiAcquireLockOrFail (&(Sock->Lock));
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockDestroyChild: Get the lock to "
       "access socket failed with %r\n", Status));
 
     return EFI_ACCESS_DENIED;
   }
 
   //
+  // Close the IP protocol.
+  //
+  gBS->CloseProtocol (
+         Tcb->IpInfo->ChildHandle,
+         &gEfiIp4ProtocolGuid,
+         ProtoData->TcpService->IpIo->Image,
+         Sock->SockHandle
+         );
+
+  if (Sock->DestroyCallback != NULL) {
+    Sock->DestroyCallback (Sock, Sock->Context);
+  }
+
+  //
+  // Retrieve the protocol installed on this sock
+  //
+  Status = gBS->OpenProtocol (
+                  Sock->SockHandle,
+                  &gEfiTcp4ProtocolGuid,
+                  &SockProtocol,
+                  Sock->DriverBinding,
+                  Sock->SockHandle,
+                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                  );
+
+  if (EFI_ERROR (Status)) {
+
+    DEBUG ((EFI_D_ERROR, "SockDestroyChild: Open protocol installed "
+      "on socket failed with %r\n", Status));
+  }
+
+  //
+  // Uninstall the protocol installed on this sock
+  // in the light of Sock->SockType
+  //
+  gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        &gEfiTcp4ProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+
+  //
   // force protocol layer to detach the PCB
   //
   Status = Sock->ProtoHandler (Sock, SOCK_DETACH, NULL);
 
   if (EFI_ERROR (Status)) {
@@ -207,10 +258,11 @@ SOCKET *
 SockCreateChild (
   IN SOCK_INIT_DATA *SockInitData
   )
 {
   SOCKET      *Sock;
+  VOID        *SockProtocol;
   EFI_STATUS  Status;
 
   //
   // create a new socket
   //
@@ -227,12 +279,11 @@ SockCreateChild (
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockCreateChild: Get the lock to "
       "access socket failed with %r\n", Status));
 
-    SockDestroy (Sock);
-    return NULL;
+    goto ERROR;
   }
   //
   // inform the protocol layer to attach the socket
   // with a new protocol control block
   //
@@ -241,15 +292,40 @@ SockCreateChild (
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockCreateChild: Protocol failed to"
       " attach a socket with %r\n", Status));
 
-    SockDestroy (Sock);
-    Sock = NULL;
+    goto ERROR;
   }
 
   return Sock;
+
+ERROR:
+
+  if (Sock->DestroyCallback != NULL) {
+    Sock->DestroyCallback (Sock, Sock->Context);
+  }
+
+  gBS->OpenProtocol (
+         Sock->SockHandle,
+         &gEfiTcp4ProtocolGuid,
+         &SockProtocol,
+         Sock->DriverBinding,
+         Sock->SockHandle,
+         EFI_OPEN_PROTOCOL_GET_PROTOCOL
+         );
+  //
+  // Uninstall the protocol installed on this sock
+  //
+  gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        &gEfiTcp4ProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+   SockDestroy (Sock);
+   return NULL;
 }
 
 
 /**
   Configure the specific socket Sock using configuration data ConfigData.
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
index 5b327af..702cae8 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
@@ -1,10 +1,10 @@
 /** @file
   Tcp request dispatcher implementation.
 
 (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
-Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
 http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -330,20 +330,10 @@ Tcp4DetachPcb (
   Tcb       = ProtoData->TcpPcb;
 
   ASSERT (Tcb != NULL);
 
   Tcp4FlushPcb (Tcb);
-
-  //
-  // Close the IP protocol.
-  //
-  gBS->CloseProtocol (
-         Tcb->IpInfo->ChildHandle,
-         &gEfiIp4ProtocolGuid,
-         ProtoData->TcpService->IpIo->Image,
-         Sk->SockHandle
-         );
   
   IpIoRemoveIp (ProtoData->TcpService->IpIo, Tcb->IpInfo);
 
   FreePool (Tcb);
 
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe.
  2017-03-15  2:27 [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe Zhang Lubo
@ 2017-03-16  1:46 ` Fu, Siyuan
  2017-03-16  3:50 ` Wu, Jiaxin
  2017-03-16  5:31 ` Ye, Ting
  2 siblings, 0 replies; 5+ messages in thread
From: Fu, Siyuan @ 2017-03-16  1:46 UTC (permalink / raw)
  To: Zhang, Lubo, edk2-devel@lists.01.org; +Cc: Wu, Jiaxin, Ye, Ting

Looks good to me.
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>

-----Original Message-----
From: Zhang, Lubo 
Sent: 2017年3月15日 10:28
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
Subject: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe.

v2: Handle error case in SockCreateChild and fix typo issue

when we destroy the socket Sock and its associated protocol control block, we need to first close the parent protocol, then remove the protocol from childHandle and last to free any data structures that allocated in CreateChild. But currently, we free the socket data (Socket ConfigureState) before removing the protocol form  the childhandle. So if the up layer want to send the tcp reset packet in it's driver binding stop function, it will failed.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <lubo.zhang@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c  | 10 +--
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c    |  3 +-
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h    |  5 +-
 MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c  | 48 +-----------  MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h  |  3 +-
 .../Universal/Network/Tcp4Dxe/SockInterface.c      | 88 ++++++++++++++++++++--
 .../Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c     | 12 +--
 7 files changed, 95 insertions(+), 74 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
index 642e453..792db5c 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
@@ -1,9 +1,9 @@
 /** @file
   The driver binding and service binding protocol for IP4 driver.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
 
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at @@ -920,11 +920,10 @@ Ip4ServiceBindingDestroyChild (
   EFI_STATUS                Status;
   IP4_SERVICE               *IpSb;
   IP4_PROTOCOL              *IpInstance;
   EFI_IP4_PROTOCOL          *Ip4;
   EFI_TPL                   OldTpl;
-  INTN                      State;
 
   if ((This == NULL) || (ChildHandle == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
@@ -958,17 +957,16 @@ Ip4ServiceBindingDestroyChild (
   // A child can be destroyed more than once. For example,
   // Ip4DriverBindingStop will destroy all of its children.
   // when UDP driver is being stopped, it will destroy all
   // the IP child it opens.
   //
-  if (IpInstance->State == IP4_STATE_DESTROY) {
+  if (IpInstance->InDestroy) {
     gBS->RestoreTPL (OldTpl);
     return EFI_SUCCESS;
   }
 
-  State             = IpInstance->State;
-  IpInstance->State = IP4_STATE_DESTROY;
+  IpInstance->InDestroy = TRUE;
 
   //
   // Close the Managed Network protocol.
   //
   gBS->CloseProtocol (
@@ -1007,10 +1005,11 @@ Ip4ServiceBindingDestroyChild (
                   &gEfiIp4ProtocolGuid,
                   &IpInstance->Ip4Proto
                   );
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
   if (EFI_ERROR (Status)) {
+    IpInstance->InDestroy = FALSE;
     goto ON_ERROR;
   }
 
   Status = Ip4CleanProtocol (IpInstance);
   if (EFI_ERROR (Status)) {
@@ -1031,10 +1030,9 @@ Ip4ServiceBindingDestroyChild (
 
   FreePool (IpInstance);
   return EFI_SUCCESS;
 
 ON_ERROR:
-  IpInstance->State = State;
   gBS->RestoreTPL (OldTpl);
 
   return Status;
 }
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
index 91f1a67..23690ff 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
@@ -1,8 +1,8 @@
 /** @file
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php
 
@@ -548,10 +548,11 @@ Ip4InitProtocol (
   ZeroMem (IpInstance, sizeof (IP4_PROTOCOL));
 
   IpInstance->Signature = IP4_PROTOCOL_SIGNATURE;
   CopyMem (&IpInstance->Ip4Proto, &mEfiIp4ProtocolTemplete, sizeof (IpInstance->Ip4Proto));
   IpInstance->State     = IP4_STATE_UNCONFIGED;
+  IpInstance->InDestroy   = FALSE;
   IpInstance->Service   = IpSb;
 
   InitializeListHead (&IpInstance->Link);
   NetMapInit  (&IpInstance->RxTokens);
   NetMapInit  (&IpInstance->TxTokens);
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
index 7a7ad9d..eee38b7 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
@@ -1,9 +1,9 @@
 /** @file
   Ip4 internal functions and type defintions.
   
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
 
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at @@ -67,11 +67,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 // is called, it becomes UNCONFIGED again. If (partly) destroyed, it  // becomes DESTROY.
 //
 #define IP4_STATE_UNCONFIGED    0
 #define IP4_STATE_CONFIGED      1
-#define IP4_STATE_DESTROY       2
 
 //
 // The state of IP4 service. It starts from UNSTARTED. It transits  // to STARTED if autoconfigure is started. If default address is  // configured, it becomes CONFIGED. and if partly destroyed, it goes @@ -134,10 +133,12 @@ struct _IP4_PROTOCOL {
 
   EFI_IP4_PROTOCOL          Ip4Proto;
   EFI_HANDLE                Handle;
   INTN                      State;
 
+  BOOLEAN                   InDestroy;                      
+
   IP4_SERVICE               *Service;
   LIST_ENTRY                Link;       // Link to all the IP protocol from the service
 
   //
   // User's transmit/receive tokens, and received/deliverd packets diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
index fc78273..0476077 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of the Socket.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -715,20 +715,12 @@ OnError:
 VOID
 SockDestroy (
   IN OUT SOCKET *Sock
   )
 {
-  VOID        *SockProtocol;
-  EFI_GUID    *ProtocolGuid;
-  EFI_STATUS  Status;
-
   ASSERT (SockStream == Sock->Type);
 
-  if (Sock->DestroyCallback != NULL) {
-    Sock->DestroyCallback (Sock, Sock->Context);
-  }
-
   //
   // Flush the completion token buffered
   // by sock and rcv, snd buffer
   //
   if (!SOCK_IS_UNCONFIGURED (Sock)) {
@@ -760,48 +752,10 @@ SockDestroy (
       );
 
     Sock->Parent = NULL;
   }
 
-  //
-  // Set the protocol guid and driver binding handle
-  // in the light of Sock->SockType
-  //
-  ProtocolGuid = &gEfiTcp4ProtocolGuid;
-
-  //
-  // Retrieve the protocol installed on this sock
-  //
-  Status = gBS->OpenProtocol (
-                  Sock->SockHandle,
-                  ProtocolGuid,
-                  &SockProtocol,
-                  Sock->DriverBinding,
-                  Sock->SockHandle,
-                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
-                  );
-
-  if (EFI_ERROR (Status)) {
-
-    DEBUG ((EFI_D_ERROR, "SockDestroy: Open protocol installed "
-      "on socket failed with %r\n", Status));
-
-    goto FreeSock;
-  }
-
-  //
-  // Uninstall the protocol installed on this sock
-  // in the light of Sock->SockType
-  //
-  gBS->UninstallMultipleProtocolInterfaces (
-        Sock->SockHandle,
-        ProtocolGuid,
-        SockProtocol,
-        NULL
-        );
-
-FreeSock:
   FreePool (Sock);
   return ;
 }
 
 
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
index 4a5a845..d4104a0 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
@@ -1,9 +1,9 @@
 /** @file
   Socket implementation header file.
 
-Copyright (c) 2005 - 2006, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2007, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -14,10 +14,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #ifndef _SOCK_IMPL_H_
 #define _SOCK_IMPL_H_
 
 #include "Socket.h"
+#include "Tcp4Main.h"
 
 /**
   Signal a event with the given status.
   
   @param Token        The token's event is to be signaled.
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
index a8bdef6..f8b535c 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
@@ -1,9 +1,9 @@
 /** @file
   Interface function of the Socket.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -142,30 +142,81 @@ SockBufferToken (
 EFI_STATUS
 SockDestroyChild (
   IN SOCKET *Sock
   )
 {
-  EFI_STATUS  Status;
+  EFI_STATUS       Status;
+  TCP4_PROTO_DATA  *ProtoData;
+  TCP_CB           *Tcb;
+  VOID             *SockProtocol;
 
   ASSERT ((Sock != NULL) && (Sock->ProtoHandler != NULL));
 
   if (Sock->InDestroy) {
     return EFI_SUCCESS;
   }
 
   Sock->InDestroy = TRUE;
 
+  ProtoData = (TCP4_PROTO_DATA *) Sock->ProtoReserved;
+  Tcb       = ProtoData->TcpPcb;
+
+  ASSERT (Tcb != NULL);
+
   Status = EfiAcquireLockOrFail (&(Sock->Lock));
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockDestroyChild: Get the lock to "
       "access socket failed with %r\n", Status));
 
     return EFI_ACCESS_DENIED;
   }
 
   //
+  // Close the IP protocol.
+  //
+  gBS->CloseProtocol (
+         Tcb->IpInfo->ChildHandle,
+         &gEfiIp4ProtocolGuid,
+         ProtoData->TcpService->IpIo->Image,
+         Sock->SockHandle
+         );
+
+  if (Sock->DestroyCallback != NULL) {
+    Sock->DestroyCallback (Sock, Sock->Context);  }
+
+  //
+  // Retrieve the protocol installed on this sock  //  Status = 
+ gBS->OpenProtocol (
+                  Sock->SockHandle,
+                  &gEfiTcp4ProtocolGuid,
+                  &SockProtocol,
+                  Sock->DriverBinding,
+                  Sock->SockHandle,
+                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                  );
+
+  if (EFI_ERROR (Status)) {
+
+    DEBUG ((EFI_D_ERROR, "SockDestroyChild: Open protocol installed "
+      "on socket failed with %r\n", Status));  }
+
+  //
+  // Uninstall the protocol installed on this sock  // in the light of 
+ Sock->SockType  //  gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        &gEfiTcp4ProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+
+  //
   // force protocol layer to detach the PCB
   //
   Status = Sock->ProtoHandler (Sock, SOCK_DETACH, NULL);
 
   if (EFI_ERROR (Status)) {
@@ -207,10 +258,11 @@ SOCKET *
 SockCreateChild (
   IN SOCK_INIT_DATA *SockInitData
   )
 {
   SOCKET      *Sock;
+  VOID        *SockProtocol;
   EFI_STATUS  Status;
 
   //
   // create a new socket
   //
@@ -227,12 +279,11 @@ SockCreateChild (
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockCreateChild: Get the lock to "
       "access socket failed with %r\n", Status));
 
-    SockDestroy (Sock);
-    return NULL;
+    goto ERROR;
   }
   //
   // inform the protocol layer to attach the socket
   // with a new protocol control block
   //
@@ -241,15 +292,40 @@ SockCreateChild (
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockCreateChild: Protocol failed to"
       " attach a socket with %r\n", Status));
 
-    SockDestroy (Sock);
-    Sock = NULL;
+    goto ERROR;
   }
 
   return Sock;
+
+ERROR:
+
+  if (Sock->DestroyCallback != NULL) {
+    Sock->DestroyCallback (Sock, Sock->Context);  }
+
+  gBS->OpenProtocol (
+         Sock->SockHandle,
+         &gEfiTcp4ProtocolGuid,
+         &SockProtocol,
+         Sock->DriverBinding,
+         Sock->SockHandle,
+         EFI_OPEN_PROTOCOL_GET_PROTOCOL
+         );
+  //
+  // Uninstall the protocol installed on this sock  //  
+ gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        &gEfiTcp4ProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+   SockDestroy (Sock);
+   return NULL;
 }
 
 
 /**
   Configure the specific socket Sock using configuration data ConfigData.
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
index 5b327af..702cae8 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
@@ -1,10 +1,10 @@
 /** @file
   Tcp request dispatcher implementation.
 
 (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR> -Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -330,20 +330,10 @@ Tcp4DetachPcb (
   Tcb       = ProtoData->TcpPcb;
 
   ASSERT (Tcb != NULL);
 
   Tcp4FlushPcb (Tcb);
-
-  //
-  // Close the IP protocol.
-  //
-  gBS->CloseProtocol (
-         Tcb->IpInfo->ChildHandle,
-         &gEfiIp4ProtocolGuid,
-         ProtoData->TcpService->IpIo->Image,
-         Sk->SockHandle
-         );
   
   IpIoRemoveIp (ProtoData->TcpService->IpIo, Tcb->IpInfo);
 
   FreePool (Tcb);
 
--
1.9.5.msysgit.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe.
  2017-03-15  2:27 [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe Zhang Lubo
  2017-03-16  1:46 ` Fu, Siyuan
@ 2017-03-16  3:50 ` Wu, Jiaxin
  2017-03-16  5:31 ` Ye, Ting
  2 siblings, 0 replies; 5+ messages in thread
From: Wu, Jiaxin @ 2017-03-16  3:50 UTC (permalink / raw)
  To: Zhang, Lubo, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan

For this patch, my comments as below:

1. The commit log should better cover the changes of Ip4Dxe but currently it's not. 

2. Intel Copyright in SockImpl.h  should be 2017, not 2007.

Other looks to me.

Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>

Thanks,
Jiaxin


> -----Original Message-----
> From: Zhang, Lubo
> Sent: Wednesday, March 15, 2017 10:28 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>
> Subject: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and
> Ip4 dxe.
> 
> v2: Handle error case in SockCreateChild and fix typo issue
> 
> when we destroy the socket Sock and its associated
> protocol control block, we need to first close the
> parent protocol, then remove the protocol from childHandle
> and last to free any data structures that allocated in
> CreateChild. But currently, we free the socket data
> (Socket ConfigureState) before removing the protocol
> form  the childhandle. So if the up layer want to send the
> tcp reset packet in it's driver binding stop function, it will failed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo <lubo.zhang@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> ---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c  | 10 +--
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c    |  3 +-
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h    |  5 +-
>  MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c  | 48 +-----------
>  MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h  |  3 +-
>  .../Universal/Network/Tcp4Dxe/SockInterface.c      | 88
> ++++++++++++++++++++--
>  .../Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c     | 12 +--
>  7 files changed, 95 insertions(+), 74 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
> index 642e453..792db5c 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
> @@ -1,9 +1,9 @@
>  /** @file
>    The driver binding and service binding protocol for IP4 driver.
> 
> -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
> 
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
> @@ -920,11 +920,10 @@ Ip4ServiceBindingDestroyChild (
>    EFI_STATUS                Status;
>    IP4_SERVICE               *IpSb;
>    IP4_PROTOCOL              *IpInstance;
>    EFI_IP4_PROTOCOL          *Ip4;
>    EFI_TPL                   OldTpl;
> -  INTN                      State;
> 
>    if ((This == NULL) || (ChildHandle == NULL)) {
>      return EFI_INVALID_PARAMETER;
>    }
> 
> @@ -958,17 +957,16 @@ Ip4ServiceBindingDestroyChild (
>    // A child can be destroyed more than once. For example,
>    // Ip4DriverBindingStop will destroy all of its children.
>    // when UDP driver is being stopped, it will destroy all
>    // the IP child it opens.
>    //
> -  if (IpInstance->State == IP4_STATE_DESTROY) {
> +  if (IpInstance->InDestroy) {
>      gBS->RestoreTPL (OldTpl);
>      return EFI_SUCCESS;
>    }
> 
> -  State             = IpInstance->State;
> -  IpInstance->State = IP4_STATE_DESTROY;
> +  IpInstance->InDestroy = TRUE;
> 
>    //
>    // Close the Managed Network protocol.
>    //
>    gBS->CloseProtocol (
> @@ -1007,10 +1005,11 @@ Ip4ServiceBindingDestroyChild (
>                    &gEfiIp4ProtocolGuid,
>                    &IpInstance->Ip4Proto
>                    );
>    OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>    if (EFI_ERROR (Status)) {
> +    IpInstance->InDestroy = FALSE;
>      goto ON_ERROR;
>    }
> 
>    Status = Ip4CleanProtocol (IpInstance);
>    if (EFI_ERROR (Status)) {
> @@ -1031,10 +1030,9 @@ Ip4ServiceBindingDestroyChild (
> 
>    FreePool (IpInstance);
>    return EFI_SUCCESS;
> 
>  ON_ERROR:
> -  IpInstance->State = State;
>    gBS->RestoreTPL (OldTpl);
> 
>    return Status;
>  }
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> index 91f1a67..23690ff 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> @@ -1,8 +1,8 @@
>  /** @file
> 
> -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -548,10 +548,11 @@ Ip4InitProtocol (
>    ZeroMem (IpInstance, sizeof (IP4_PROTOCOL));
> 
>    IpInstance->Signature = IP4_PROTOCOL_SIGNATURE;
>    CopyMem (&IpInstance->Ip4Proto, &mEfiIp4ProtocolTemplete, sizeof
> (IpInstance->Ip4Proto));
>    IpInstance->State     = IP4_STATE_UNCONFIGED;
> +  IpInstance->InDestroy   = FALSE;
>    IpInstance->Service   = IpSb;
> 
>    InitializeListHead (&IpInstance->Link);
>    NetMapInit  (&IpInstance->RxTokens);
>    NetMapInit  (&IpInstance->TxTokens);
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
> index 7a7ad9d..eee38b7 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
> @@ -1,9 +1,9 @@
>  /** @file
>    Ip4 internal functions and type defintions.
> 
> -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
> 
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
> @@ -67,11 +67,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>  // is called, it becomes UNCONFIGED again. If (partly) destroyed, it
>  // becomes DESTROY.
>  //
>  #define IP4_STATE_UNCONFIGED    0
>  #define IP4_STATE_CONFIGED      1
> -#define IP4_STATE_DESTROY       2
> 
>  //
>  // The state of IP4 service. It starts from UNSTARTED. It transits
>  // to STARTED if autoconfigure is started. If default address is
>  // configured, it becomes CONFIGED. and if partly destroyed, it goes
> @@ -134,10 +133,12 @@ struct _IP4_PROTOCOL {
> 
>    EFI_IP4_PROTOCOL          Ip4Proto;
>    EFI_HANDLE                Handle;
>    INTN                      State;
> 
> +  BOOLEAN                   InDestroy;
> +
>    IP4_SERVICE               *Service;
>    LIST_ENTRY                Link;       // Link to all the IP protocol from the service
> 
>    //
>    // User's transmit/receive tokens, and received/deliverd packets
> diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
> b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
> index fc78273..0476077 100644
> --- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
> +++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
> @@ -1,9 +1,9 @@
>  /** @file
>    Implementation of the Socket.
> 
> -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php<BR>
> 
> @@ -715,20 +715,12 @@ OnError:
>  VOID
>  SockDestroy (
>    IN OUT SOCKET *Sock
>    )
>  {
> -  VOID        *SockProtocol;
> -  EFI_GUID    *ProtocolGuid;
> -  EFI_STATUS  Status;
> -
>    ASSERT (SockStream == Sock->Type);
> 
> -  if (Sock->DestroyCallback != NULL) {
> -    Sock->DestroyCallback (Sock, Sock->Context);
> -  }
> -
>    //
>    // Flush the completion token buffered
>    // by sock and rcv, snd buffer
>    //
>    if (!SOCK_IS_UNCONFIGURED (Sock)) {
> @@ -760,48 +752,10 @@ SockDestroy (
>        );
> 
>      Sock->Parent = NULL;
>    }
> 
> -  //
> -  // Set the protocol guid and driver binding handle
> -  // in the light of Sock->SockType
> -  //
> -  ProtocolGuid = &gEfiTcp4ProtocolGuid;
> -
> -  //
> -  // Retrieve the protocol installed on this sock
> -  //
> -  Status = gBS->OpenProtocol (
> -                  Sock->SockHandle,
> -                  ProtocolGuid,
> -                  &SockProtocol,
> -                  Sock->DriverBinding,
> -                  Sock->SockHandle,
> -                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
> -                  );
> -
> -  if (EFI_ERROR (Status)) {
> -
> -    DEBUG ((EFI_D_ERROR, "SockDestroy: Open protocol installed "
> -      "on socket failed with %r\n", Status));
> -
> -    goto FreeSock;
> -  }
> -
> -  //
> -  // Uninstall the protocol installed on this sock
> -  // in the light of Sock->SockType
> -  //
> -  gBS->UninstallMultipleProtocolInterfaces (
> -        Sock->SockHandle,
> -        ProtocolGuid,
> -        SockProtocol,
> -        NULL
> -        );
> -
> -FreeSock:
>    FreePool (Sock);
>    return ;
>  }
> 
> 
> diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
> b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
> index 4a5a845..d4104a0 100644
> --- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
> +++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
> @@ -1,9 +1,9 @@
>  /** @file
>    Socket implementation header file.
> 
> -Copyright (c) 2005 - 2006, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2007, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php<BR>
> 
> @@ -14,10 +14,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
> 
>  #ifndef _SOCK_IMPL_H_
>  #define _SOCK_IMPL_H_
> 
>  #include "Socket.h"
> +#include "Tcp4Main.h"
> 
>  /**
>    Signal a event with the given status.
> 
>    @param Token        The token's event is to be signaled.
> diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
> b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
> index a8bdef6..f8b535c 100644
> --- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
> +++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
> @@ -1,9 +1,9 @@
>  /** @file
>    Interface function of the Socket.
> 
> -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php<BR>
> 
> @@ -142,30 +142,81 @@ SockBufferToken (
>  EFI_STATUS
>  SockDestroyChild (
>    IN SOCKET *Sock
>    )
>  {
> -  EFI_STATUS  Status;
> +  EFI_STATUS       Status;
> +  TCP4_PROTO_DATA  *ProtoData;
> +  TCP_CB           *Tcb;
> +  VOID             *SockProtocol;
> 
>    ASSERT ((Sock != NULL) && (Sock->ProtoHandler != NULL));
> 
>    if (Sock->InDestroy) {
>      return EFI_SUCCESS;
>    }
> 
>    Sock->InDestroy = TRUE;
> 
> +  ProtoData = (TCP4_PROTO_DATA *) Sock->ProtoReserved;
> +  Tcb       = ProtoData->TcpPcb;
> +
> +  ASSERT (Tcb != NULL);
> +
>    Status = EfiAcquireLockOrFail (&(Sock->Lock));
>    if (EFI_ERROR (Status)) {
> 
>      DEBUG ((EFI_D_ERROR, "SockDestroyChild: Get the lock to "
>        "access socket failed with %r\n", Status));
> 
>      return EFI_ACCESS_DENIED;
>    }
> 
>    //
> +  // Close the IP protocol.
> +  //
> +  gBS->CloseProtocol (
> +         Tcb->IpInfo->ChildHandle,
> +         &gEfiIp4ProtocolGuid,
> +         ProtoData->TcpService->IpIo->Image,
> +         Sock->SockHandle
> +         );
> +
> +  if (Sock->DestroyCallback != NULL) {
> +    Sock->DestroyCallback (Sock, Sock->Context);
> +  }
> +
> +  //
> +  // Retrieve the protocol installed on this sock
> +  //
> +  Status = gBS->OpenProtocol (
> +                  Sock->SockHandle,
> +                  &gEfiTcp4ProtocolGuid,
> +                  &SockProtocol,
> +                  Sock->DriverBinding,
> +                  Sock->SockHandle,
> +                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
> +                  );
> +
> +  if (EFI_ERROR (Status)) {
> +
> +    DEBUG ((EFI_D_ERROR, "SockDestroyChild: Open protocol installed "
> +      "on socket failed with %r\n", Status));
> +  }
> +
> +  //
> +  // Uninstall the protocol installed on this sock
> +  // in the light of Sock->SockType
> +  //
> +  gBS->UninstallMultipleProtocolInterfaces (
> +        Sock->SockHandle,
> +        &gEfiTcp4ProtocolGuid,
> +        SockProtocol,
> +        NULL
> +        );
> +
> +  //
>    // force protocol layer to detach the PCB
>    //
>    Status = Sock->ProtoHandler (Sock, SOCK_DETACH, NULL);
> 
>    if (EFI_ERROR (Status)) {
> @@ -207,10 +258,11 @@ SOCKET *
>  SockCreateChild (
>    IN SOCK_INIT_DATA *SockInitData
>    )
>  {
>    SOCKET      *Sock;
> +  VOID        *SockProtocol;
>    EFI_STATUS  Status;
> 
>    //
>    // create a new socket
>    //
> @@ -227,12 +279,11 @@ SockCreateChild (
>    if (EFI_ERROR (Status)) {
> 
>      DEBUG ((EFI_D_ERROR, "SockCreateChild: Get the lock to "
>        "access socket failed with %r\n", Status));
> 
> -    SockDestroy (Sock);
> -    return NULL;
> +    goto ERROR;
>    }
>    //
>    // inform the protocol layer to attach the socket
>    // with a new protocol control block
>    //
> @@ -241,15 +292,40 @@ SockCreateChild (
>    if (EFI_ERROR (Status)) {
> 
>      DEBUG ((EFI_D_ERROR, "SockCreateChild: Protocol failed to"
>        " attach a socket with %r\n", Status));
> 
> -    SockDestroy (Sock);
> -    Sock = NULL;
> +    goto ERROR;
>    }
> 
>    return Sock;
> +
> +ERROR:
> +
> +  if (Sock->DestroyCallback != NULL) {
> +    Sock->DestroyCallback (Sock, Sock->Context);
> +  }
> +
> +  gBS->OpenProtocol (
> +         Sock->SockHandle,
> +         &gEfiTcp4ProtocolGuid,
> +         &SockProtocol,
> +         Sock->DriverBinding,
> +         Sock->SockHandle,
> +         EFI_OPEN_PROTOCOL_GET_PROTOCOL
> +         );
> +  //
> +  // Uninstall the protocol installed on this sock
> +  //
> +  gBS->UninstallMultipleProtocolInterfaces (
> +        Sock->SockHandle,
> +        &gEfiTcp4ProtocolGuid,
> +        SockProtocol,
> +        NULL
> +        );
> +   SockDestroy (Sock);
> +   return NULL;
>  }
> 
> 
>  /**
>    Configure the specific socket Sock using configuration data ConfigData.
> diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
> b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
> index 5b327af..702cae8 100644
> --- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
> +++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
> @@ -1,10 +1,10 @@
>  /** @file
>    Tcp request dispatcher implementation.
> 
>  (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> -Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php<BR>
> 
> @@ -330,20 +330,10 @@ Tcp4DetachPcb (
>    Tcb       = ProtoData->TcpPcb;
> 
>    ASSERT (Tcb != NULL);
> 
>    Tcp4FlushPcb (Tcb);
> -
> -  //
> -  // Close the IP protocol.
> -  //
> -  gBS->CloseProtocol (
> -         Tcb->IpInfo->ChildHandle,
> -         &gEfiIp4ProtocolGuid,
> -         ProtoData->TcpService->IpIo->Image,
> -         Sk->SockHandle
> -         );
> 
>    IpIoRemoveIp (ProtoData->TcpService->IpIo, Tcb->IpInfo);
> 
>    FreePool (Tcb);
> 
> --
> 1.9.5.msysgit.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe.
  2017-03-15  2:27 [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe Zhang Lubo
  2017-03-16  1:46 ` Fu, Siyuan
  2017-03-16  3:50 ` Wu, Jiaxin
@ 2017-03-16  5:31 ` Ye, Ting
  2017-03-16  5:36   ` Zhang, Lubo
  2 siblings, 1 reply; 5+ messages in thread
From: Ye, Ting @ 2017-03-16  5:31 UTC (permalink / raw)
  To: Zhang, Lubo, edk2-devel@lists.01.org; +Cc: Wu, Jiaxin, Fu, Siyuan

Hi Lubo,

Please update the comment for IP4_STATE_DESTROY in Ip4Impl.h and other possible places, since the patch removed the value. 
Also update CopyRight year in SockImpl.h before check-in. Others are good to me.

Reviewed-by: Ye Ting <ting.ye@intel.com>

Best Regards,
Ting


-----Original Message-----
From: Zhang, Lubo 
Sent: Wednesday, March 15, 2017 10:28 AM
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
Subject: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe.

v2: Handle error case in SockCreateChild and fix typo issue

when we destroy the socket Sock and its associated protocol control block, we need to first close the parent protocol, then remove the protocol from childHandle and last to free any data structures that allocated in CreateChild. But currently, we free the socket data (Socket ConfigureState) before removing the protocol form  the childhandle. So if the up layer want to send the tcp reset packet in it's driver binding stop function, it will failed.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <lubo.zhang@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c  | 10 +--
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c    |  3 +-
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h    |  5 +-
 MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c  | 48 +-----------  MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h  |  3 +-
 .../Universal/Network/Tcp4Dxe/SockInterface.c      | 88 ++++++++++++++++++++--
 .../Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c     | 12 +--
 7 files changed, 95 insertions(+), 74 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
index 642e453..792db5c 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
@@ -1,9 +1,9 @@
 /** @file
   The driver binding and service binding protocol for IP4 driver.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
 
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at @@ -920,11 +920,10 @@ Ip4ServiceBindingDestroyChild (
   EFI_STATUS                Status;
   IP4_SERVICE               *IpSb;
   IP4_PROTOCOL              *IpInstance;
   EFI_IP4_PROTOCOL          *Ip4;
   EFI_TPL                   OldTpl;
-  INTN                      State;
 
   if ((This == NULL) || (ChildHandle == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
@@ -958,17 +957,16 @@ Ip4ServiceBindingDestroyChild (
   // A child can be destroyed more than once. For example,
   // Ip4DriverBindingStop will destroy all of its children.
   // when UDP driver is being stopped, it will destroy all
   // the IP child it opens.
   //
-  if (IpInstance->State == IP4_STATE_DESTROY) {
+  if (IpInstance->InDestroy) {
     gBS->RestoreTPL (OldTpl);
     return EFI_SUCCESS;
   }
 
-  State             = IpInstance->State;
-  IpInstance->State = IP4_STATE_DESTROY;
+  IpInstance->InDestroy = TRUE;
 
   //
   // Close the Managed Network protocol.
   //
   gBS->CloseProtocol (
@@ -1007,10 +1005,11 @@ Ip4ServiceBindingDestroyChild (
                   &gEfiIp4ProtocolGuid,
                   &IpInstance->Ip4Proto
                   );
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
   if (EFI_ERROR (Status)) {
+    IpInstance->InDestroy = FALSE;
     goto ON_ERROR;
   }
 
   Status = Ip4CleanProtocol (IpInstance);
   if (EFI_ERROR (Status)) {
@@ -1031,10 +1030,9 @@ Ip4ServiceBindingDestroyChild (
 
   FreePool (IpInstance);
   return EFI_SUCCESS;
 
 ON_ERROR:
-  IpInstance->State = State;
   gBS->RestoreTPL (OldTpl);
 
   return Status;
 }
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
index 91f1a67..23690ff 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
@@ -1,8 +1,8 @@
 /** @file
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php
 
@@ -548,10 +548,11 @@ Ip4InitProtocol (
   ZeroMem (IpInstance, sizeof (IP4_PROTOCOL));
 
   IpInstance->Signature = IP4_PROTOCOL_SIGNATURE;
   CopyMem (&IpInstance->Ip4Proto, &mEfiIp4ProtocolTemplete, sizeof (IpInstance->Ip4Proto));
   IpInstance->State     = IP4_STATE_UNCONFIGED;
+  IpInstance->InDestroy   = FALSE;
   IpInstance->Service   = IpSb;
 
   InitializeListHead (&IpInstance->Link);
   NetMapInit  (&IpInstance->RxTokens);
   NetMapInit  (&IpInstance->TxTokens);
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
index 7a7ad9d..eee38b7 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
@@ -1,9 +1,9 @@
 /** @file
   Ip4 internal functions and type defintions.
   
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
 
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at @@ -67,11 +67,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 // is called, it becomes UNCONFIGED again. If (partly) destroyed, it  // becomes DESTROY.
 //
 #define IP4_STATE_UNCONFIGED    0
 #define IP4_STATE_CONFIGED      1
-#define IP4_STATE_DESTROY       2
 
 //
 // The state of IP4 service. It starts from UNSTARTED. It transits  // to STARTED if autoconfigure is started. If default address is  // configured, it becomes CONFIGED. and if partly destroyed, it goes @@ -134,10 +133,12 @@ struct _IP4_PROTOCOL {
 
   EFI_IP4_PROTOCOL          Ip4Proto;
   EFI_HANDLE                Handle;
   INTN                      State;
 
+  BOOLEAN                   InDestroy;                      
+
   IP4_SERVICE               *Service;
   LIST_ENTRY                Link;       // Link to all the IP protocol from the service
 
   //
   // User's transmit/receive tokens, and received/deliverd packets diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
index fc78273..0476077 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of the Socket.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -715,20 +715,12 @@ OnError:
 VOID
 SockDestroy (
   IN OUT SOCKET *Sock
   )
 {
-  VOID        *SockProtocol;
-  EFI_GUID    *ProtocolGuid;
-  EFI_STATUS  Status;
-
   ASSERT (SockStream == Sock->Type);
 
-  if (Sock->DestroyCallback != NULL) {
-    Sock->DestroyCallback (Sock, Sock->Context);
-  }
-
   //
   // Flush the completion token buffered
   // by sock and rcv, snd buffer
   //
   if (!SOCK_IS_UNCONFIGURED (Sock)) {
@@ -760,48 +752,10 @@ SockDestroy (
       );
 
     Sock->Parent = NULL;
   }
 
-  //
-  // Set the protocol guid and driver binding handle
-  // in the light of Sock->SockType
-  //
-  ProtocolGuid = &gEfiTcp4ProtocolGuid;
-
-  //
-  // Retrieve the protocol installed on this sock
-  //
-  Status = gBS->OpenProtocol (
-                  Sock->SockHandle,
-                  ProtocolGuid,
-                  &SockProtocol,
-                  Sock->DriverBinding,
-                  Sock->SockHandle,
-                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
-                  );
-
-  if (EFI_ERROR (Status)) {
-
-    DEBUG ((EFI_D_ERROR, "SockDestroy: Open protocol installed "
-      "on socket failed with %r\n", Status));
-
-    goto FreeSock;
-  }
-
-  //
-  // Uninstall the protocol installed on this sock
-  // in the light of Sock->SockType
-  //
-  gBS->UninstallMultipleProtocolInterfaces (
-        Sock->SockHandle,
-        ProtocolGuid,
-        SockProtocol,
-        NULL
-        );
-
-FreeSock:
   FreePool (Sock);
   return ;
 }
 
 
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
index 4a5a845..d4104a0 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
@@ -1,9 +1,9 @@
 /** @file
   Socket implementation header file.
 
-Copyright (c) 2005 - 2006, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2007, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -14,10 +14,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #ifndef _SOCK_IMPL_H_
 #define _SOCK_IMPL_H_
 
 #include "Socket.h"
+#include "Tcp4Main.h"
 
 /**
   Signal a event with the given status.
   
   @param Token        The token's event is to be signaled.
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
index a8bdef6..f8b535c 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
@@ -1,9 +1,9 @@
 /** @file
   Interface function of the Socket.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -142,30 +142,81 @@ SockBufferToken (
 EFI_STATUS
 SockDestroyChild (
   IN SOCKET *Sock
   )
 {
-  EFI_STATUS  Status;
+  EFI_STATUS       Status;
+  TCP4_PROTO_DATA  *ProtoData;
+  TCP_CB           *Tcb;
+  VOID             *SockProtocol;
 
   ASSERT ((Sock != NULL) && (Sock->ProtoHandler != NULL));
 
   if (Sock->InDestroy) {
     return EFI_SUCCESS;
   }
 
   Sock->InDestroy = TRUE;
 
+  ProtoData = (TCP4_PROTO_DATA *) Sock->ProtoReserved;
+  Tcb       = ProtoData->TcpPcb;
+
+  ASSERT (Tcb != NULL);
+
   Status = EfiAcquireLockOrFail (&(Sock->Lock));
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockDestroyChild: Get the lock to "
       "access socket failed with %r\n", Status));
 
     return EFI_ACCESS_DENIED;
   }
 
   //
+  // Close the IP protocol.
+  //
+  gBS->CloseProtocol (
+         Tcb->IpInfo->ChildHandle,
+         &gEfiIp4ProtocolGuid,
+         ProtoData->TcpService->IpIo->Image,
+         Sock->SockHandle
+         );
+
+  if (Sock->DestroyCallback != NULL) {
+    Sock->DestroyCallback (Sock, Sock->Context);  }
+
+  //
+  // Retrieve the protocol installed on this sock  //  Status = 
+ gBS->OpenProtocol (
+                  Sock->SockHandle,
+                  &gEfiTcp4ProtocolGuid,
+                  &SockProtocol,
+                  Sock->DriverBinding,
+                  Sock->SockHandle,
+                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                  );
+
+  if (EFI_ERROR (Status)) {
+
+    DEBUG ((EFI_D_ERROR, "SockDestroyChild: Open protocol installed "
+      "on socket failed with %r\n", Status));  }
+
+  //
+  // Uninstall the protocol installed on this sock  // in the light of 
+ Sock->SockType  //  gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        &gEfiTcp4ProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+
+  //
   // force protocol layer to detach the PCB
   //
   Status = Sock->ProtoHandler (Sock, SOCK_DETACH, NULL);
 
   if (EFI_ERROR (Status)) {
@@ -207,10 +258,11 @@ SOCKET *
 SockCreateChild (
   IN SOCK_INIT_DATA *SockInitData
   )
 {
   SOCKET      *Sock;
+  VOID        *SockProtocol;
   EFI_STATUS  Status;
 
   //
   // create a new socket
   //
@@ -227,12 +279,11 @@ SockCreateChild (
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockCreateChild: Get the lock to "
       "access socket failed with %r\n", Status));
 
-    SockDestroy (Sock);
-    return NULL;
+    goto ERROR;
   }
   //
   // inform the protocol layer to attach the socket
   // with a new protocol control block
   //
@@ -241,15 +292,40 @@ SockCreateChild (
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockCreateChild: Protocol failed to"
       " attach a socket with %r\n", Status));
 
-    SockDestroy (Sock);
-    Sock = NULL;
+    goto ERROR;
   }
 
   return Sock;
+
+ERROR:
+
+  if (Sock->DestroyCallback != NULL) {
+    Sock->DestroyCallback (Sock, Sock->Context);  }
+
+  gBS->OpenProtocol (
+         Sock->SockHandle,
+         &gEfiTcp4ProtocolGuid,
+         &SockProtocol,
+         Sock->DriverBinding,
+         Sock->SockHandle,
+         EFI_OPEN_PROTOCOL_GET_PROTOCOL
+         );
+  //
+  // Uninstall the protocol installed on this sock  //  
+ gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        &gEfiTcp4ProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+   SockDestroy (Sock);
+   return NULL;
 }
 
 
 /**
   Configure the specific socket Sock using configuration data ConfigData.
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
index 5b327af..702cae8 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
@@ -1,10 +1,10 @@
 /** @file
   Tcp request dispatcher implementation.
 
 (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR> -Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -330,20 +330,10 @@ Tcp4DetachPcb (
   Tcb       = ProtoData->TcpPcb;
 
   ASSERT (Tcb != NULL);
 
   Tcp4FlushPcb (Tcb);
-
-  //
-  // Close the IP protocol.
-  //
-  gBS->CloseProtocol (
-         Tcb->IpInfo->ChildHandle,
-         &gEfiIp4ProtocolGuid,
-         ProtoData->TcpService->IpIo->Image,
-         Sk->SockHandle
-         );
   
   IpIoRemoveIp (ProtoData->TcpService->IpIo, Tcb->IpInfo);
 
   FreePool (Tcb);
 
--
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe.
  2017-03-16  5:31 ` Ye, Ting
@ 2017-03-16  5:36   ` Zhang, Lubo
  0 siblings, 0 replies; 5+ messages in thread
From: Zhang, Lubo @ 2017-03-16  5:36 UTC (permalink / raw)
  To: Ye, Ting, edk2-devel@lists.01.org; +Cc: Wu, Jiaxin, Fu, Siyuan

Go it, thanks for your comments

Best Regards
Lubo 

-----Original Message-----
From: Ye, Ting 
Sent: Thursday, March 16, 2017 1:31 PM
To: Zhang, Lubo <lubo.zhang@intel.com>; edk2-devel@lists.01.org
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
Subject: RE: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe.

Hi Lubo,

Please update the comment for IP4_STATE_DESTROY in Ip4Impl.h and other possible places, since the patch removed the value. 
Also update CopyRight year in SockImpl.h before check-in. Others are good to me.

Reviewed-by: Ye Ting <ting.ye@intel.com>

Best Regards,
Ting


-----Original Message-----
From: Zhang, Lubo 
Sent: Wednesday, March 15, 2017 10:28 AM
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
Subject: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe.

v2: Handle error case in SockCreateChild and fix typo issue

when we destroy the socket Sock and its associated protocol control block, we need to first close the parent protocol, then remove the protocol from childHandle and last to free any data structures that allocated in CreateChild. But currently, we free the socket data (Socket ConfigureState) before removing the protocol form  the childhandle. So if the up layer want to send the tcp reset packet in it's driver binding stop function, it will failed.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <lubo.zhang@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c  | 10 +--
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c    |  3 +-
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h    |  5 +-
 MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c  | 48 +-----------  MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h  |  3 +-
 .../Universal/Network/Tcp4Dxe/SockInterface.c      | 88 ++++++++++++++++++++--
 .../Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c     | 12 +--
 7 files changed, 95 insertions(+), 74 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
index 642e453..792db5c 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
@@ -1,9 +1,9 @@
 /** @file
   The driver binding and service binding protocol for IP4 driver.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
 
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at @@ -920,11 +920,10 @@ Ip4ServiceBindingDestroyChild (
   EFI_STATUS                Status;
   IP4_SERVICE               *IpSb;
   IP4_PROTOCOL              *IpInstance;
   EFI_IP4_PROTOCOL          *Ip4;
   EFI_TPL                   OldTpl;
-  INTN                      State;
 
   if ((This == NULL) || (ChildHandle == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
@@ -958,17 +957,16 @@ Ip4ServiceBindingDestroyChild (
   // A child can be destroyed more than once. For example,
   // Ip4DriverBindingStop will destroy all of its children.
   // when UDP driver is being stopped, it will destroy all
   // the IP child it opens.
   //
-  if (IpInstance->State == IP4_STATE_DESTROY) {
+  if (IpInstance->InDestroy) {
     gBS->RestoreTPL (OldTpl);
     return EFI_SUCCESS;
   }
 
-  State             = IpInstance->State;
-  IpInstance->State = IP4_STATE_DESTROY;
+  IpInstance->InDestroy = TRUE;
 
   //
   // Close the Managed Network protocol.
   //
   gBS->CloseProtocol (
@@ -1007,10 +1005,11 @@ Ip4ServiceBindingDestroyChild (
                   &gEfiIp4ProtocolGuid,
                   &IpInstance->Ip4Proto
                   );
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
   if (EFI_ERROR (Status)) {
+    IpInstance->InDestroy = FALSE;
     goto ON_ERROR;
   }
 
   Status = Ip4CleanProtocol (IpInstance);
   if (EFI_ERROR (Status)) {
@@ -1031,10 +1030,9 @@ Ip4ServiceBindingDestroyChild (
 
   FreePool (IpInstance);
   return EFI_SUCCESS;
 
 ON_ERROR:
-  IpInstance->State = State;
   gBS->RestoreTPL (OldTpl);
 
   return Status;
 }
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
index 91f1a67..23690ff 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
@@ -1,8 +1,8 @@
 /** @file
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php
 
@@ -548,10 +548,11 @@ Ip4InitProtocol (
   ZeroMem (IpInstance, sizeof (IP4_PROTOCOL));
 
   IpInstance->Signature = IP4_PROTOCOL_SIGNATURE;
   CopyMem (&IpInstance->Ip4Proto, &mEfiIp4ProtocolTemplete, sizeof (IpInstance->Ip4Proto));
   IpInstance->State     = IP4_STATE_UNCONFIGED;
+  IpInstance->InDestroy   = FALSE;
   IpInstance->Service   = IpSb;
 
   InitializeListHead (&IpInstance->Link);
   NetMapInit  (&IpInstance->RxTokens);
   NetMapInit  (&IpInstance->TxTokens);
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
index 7a7ad9d..eee38b7 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.h
@@ -1,9 +1,9 @@
 /** @file
   Ip4 internal functions and type defintions.
   
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
 
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at @@ -67,11 +67,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 // is called, it becomes UNCONFIGED again. If (partly) destroyed, it  // becomes DESTROY.
 //
 #define IP4_STATE_UNCONFIGED    0
 #define IP4_STATE_CONFIGED      1
-#define IP4_STATE_DESTROY       2
 
 //
 // The state of IP4 service. It starts from UNSTARTED. It transits  // to STARTED if autoconfigure is started. If default address is  // configured, it becomes CONFIGED. and if partly destroyed, it goes @@ -134,10 +133,12 @@ struct _IP4_PROTOCOL {
 
   EFI_IP4_PROTOCOL          Ip4Proto;
   EFI_HANDLE                Handle;
   INTN                      State;
 
+  BOOLEAN                   InDestroy;                      
+
   IP4_SERVICE               *Service;
   LIST_ENTRY                Link;       // Link to all the IP protocol from the service
 
   //
   // User's transmit/receive tokens, and received/deliverd packets diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
index fc78273..0476077 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of the Socket.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -715,20 +715,12 @@ OnError:
 VOID
 SockDestroy (
   IN OUT SOCKET *Sock
   )
 {
-  VOID        *SockProtocol;
-  EFI_GUID    *ProtocolGuid;
-  EFI_STATUS  Status;
-
   ASSERT (SockStream == Sock->Type);
 
-  if (Sock->DestroyCallback != NULL) {
-    Sock->DestroyCallback (Sock, Sock->Context);
-  }
-
   //
   // Flush the completion token buffered
   // by sock and rcv, snd buffer
   //
   if (!SOCK_IS_UNCONFIGURED (Sock)) {
@@ -760,48 +752,10 @@ SockDestroy (
       );
 
     Sock->Parent = NULL;
   }
 
-  //
-  // Set the protocol guid and driver binding handle
-  // in the light of Sock->SockType
-  //
-  ProtocolGuid = &gEfiTcp4ProtocolGuid;
-
-  //
-  // Retrieve the protocol installed on this sock
-  //
-  Status = gBS->OpenProtocol (
-                  Sock->SockHandle,
-                  ProtocolGuid,
-                  &SockProtocol,
-                  Sock->DriverBinding,
-                  Sock->SockHandle,
-                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
-                  );
-
-  if (EFI_ERROR (Status)) {
-
-    DEBUG ((EFI_D_ERROR, "SockDestroy: Open protocol installed "
-      "on socket failed with %r\n", Status));
-
-    goto FreeSock;
-  }
-
-  //
-  // Uninstall the protocol installed on this sock
-  // in the light of Sock->SockType
-  //
-  gBS->UninstallMultipleProtocolInterfaces (
-        Sock->SockHandle,
-        ProtocolGuid,
-        SockProtocol,
-        NULL
-        );
-
-FreeSock:
   FreePool (Sock);
   return ;
 }
 
 
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
index 4a5a845..d4104a0 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h
@@ -1,9 +1,9 @@
 /** @file
   Socket implementation header file.
 
-Copyright (c) 2005 - 2006, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2007, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -14,10 +14,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #ifndef _SOCK_IMPL_H_
 #define _SOCK_IMPL_H_
 
 #include "Socket.h"
+#include "Tcp4Main.h"
 
 /**
   Signal a event with the given status.
   
   @param Token        The token's event is to be signaled.
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
index a8bdef6..f8b535c 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c
@@ -1,9 +1,9 @@
 /** @file
   Interface function of the Socket.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -142,30 +142,81 @@ SockBufferToken (
 EFI_STATUS
 SockDestroyChild (
   IN SOCKET *Sock
   )
 {
-  EFI_STATUS  Status;
+  EFI_STATUS       Status;
+  TCP4_PROTO_DATA  *ProtoData;
+  TCP_CB           *Tcb;
+  VOID             *SockProtocol;
 
   ASSERT ((Sock != NULL) && (Sock->ProtoHandler != NULL));
 
   if (Sock->InDestroy) {
     return EFI_SUCCESS;
   }
 
   Sock->InDestroy = TRUE;
 
+  ProtoData = (TCP4_PROTO_DATA *) Sock->ProtoReserved;
+  Tcb       = ProtoData->TcpPcb;
+
+  ASSERT (Tcb != NULL);
+
   Status = EfiAcquireLockOrFail (&(Sock->Lock));
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockDestroyChild: Get the lock to "
       "access socket failed with %r\n", Status));
 
     return EFI_ACCESS_DENIED;
   }
 
   //
+  // Close the IP protocol.
+  //
+  gBS->CloseProtocol (
+         Tcb->IpInfo->ChildHandle,
+         &gEfiIp4ProtocolGuid,
+         ProtoData->TcpService->IpIo->Image,
+         Sock->SockHandle
+         );
+
+  if (Sock->DestroyCallback != NULL) {
+    Sock->DestroyCallback (Sock, Sock->Context);  }
+
+  //
+  // Retrieve the protocol installed on this sock  //  Status = 
+ gBS->OpenProtocol (
+                  Sock->SockHandle,
+                  &gEfiTcp4ProtocolGuid,
+                  &SockProtocol,
+                  Sock->DriverBinding,
+                  Sock->SockHandle,
+                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                  );
+
+  if (EFI_ERROR (Status)) {
+
+    DEBUG ((EFI_D_ERROR, "SockDestroyChild: Open protocol installed "
+      "on socket failed with %r\n", Status));  }
+
+  //
+  // Uninstall the protocol installed on this sock  // in the light of 
+ Sock->SockType  //  gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        &gEfiTcp4ProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+
+  //
   // force protocol layer to detach the PCB
   //
   Status = Sock->ProtoHandler (Sock, SOCK_DETACH, NULL);
 
   if (EFI_ERROR (Status)) {
@@ -207,10 +258,11 @@ SOCKET *
 SockCreateChild (
   IN SOCK_INIT_DATA *SockInitData
   )
 {
   SOCKET      *Sock;
+  VOID        *SockProtocol;
   EFI_STATUS  Status;
 
   //
   // create a new socket
   //
@@ -227,12 +279,11 @@ SockCreateChild (
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockCreateChild: Get the lock to "
       "access socket failed with %r\n", Status));
 
-    SockDestroy (Sock);
-    return NULL;
+    goto ERROR;
   }
   //
   // inform the protocol layer to attach the socket
   // with a new protocol control block
   //
@@ -241,15 +292,40 @@ SockCreateChild (
   if (EFI_ERROR (Status)) {
 
     DEBUG ((EFI_D_ERROR, "SockCreateChild: Protocol failed to"
       " attach a socket with %r\n", Status));
 
-    SockDestroy (Sock);
-    Sock = NULL;
+    goto ERROR;
   }
 
   return Sock;
+
+ERROR:
+
+  if (Sock->DestroyCallback != NULL) {
+    Sock->DestroyCallback (Sock, Sock->Context);  }
+
+  gBS->OpenProtocol (
+         Sock->SockHandle,
+         &gEfiTcp4ProtocolGuid,
+         &SockProtocol,
+         Sock->DriverBinding,
+         Sock->SockHandle,
+         EFI_OPEN_PROTOCOL_GET_PROTOCOL
+         );
+  //
+  // Uninstall the protocol installed on this sock  //  
+ gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        &gEfiTcp4ProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+   SockDestroy (Sock);
+   return NULL;
 }
 
 
 /**
   Configure the specific socket Sock using configuration data ConfigData.
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
index 5b327af..702cae8 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c
@@ -1,10 +1,10 @@
 /** @file
   Tcp request dispatcher implementation.
 
 (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR> -Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials  are licensed and made available under the terms and conditions of the BSD License  which accompanies this distribution.  The full text of the license may be found at  http://opensource.org/licenses/bsd-license.php<BR>
 
@@ -330,20 +330,10 @@ Tcp4DetachPcb (
   Tcb       = ProtoData->TcpPcb;
 
   ASSERT (Tcb != NULL);
 
   Tcp4FlushPcb (Tcb);
-
-  //
-  // Close the IP protocol.
-  //
-  gBS->CloseProtocol (
-         Tcb->IpInfo->ChildHandle,
-         &gEfiIp4ProtocolGuid,
-         ProtoData->TcpService->IpIo->Image,
-         Sk->SockHandle
-         );
   
   IpIoRemoveIp (ProtoData->TcpService->IpIo, Tcb->IpInfo);
 
   FreePool (Tcb);
 
--
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-03-16  5:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-15  2:27 [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe Zhang Lubo
2017-03-16  1:46 ` Fu, Siyuan
2017-03-16  3:50 ` Wu, Jiaxin
2017-03-16  5:31 ` Ye, Ting
2017-03-16  5:36   ` Zhang, Lubo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox