public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V2] NetworkPkg: Fix service binding issue in TCP dxe.
@ 2017-03-15  2:27 Zhang Lubo
  2017-03-16  1:46 ` Fu, Siyuan
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ 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
perform the driverbing stop to abort tcp session and send the tcp reset
packet, 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>
---
 NetworkPkg/TcpDxe/SockImpl.c      |  56 +-------------------
 NetworkPkg/TcpDxe/SockImpl.h      |   3 +-
 NetworkPkg/TcpDxe/SockInterface.c | 108 +++++++++++++++++++++++++++++++++++---
 NetworkPkg/TcpDxe/TcpDispatcher.c |  19 +------
 4 files changed, 104 insertions(+), 82 deletions(-)

diff --git a/NetworkPkg/TcpDxe/SockImpl.c b/NetworkPkg/TcpDxe/SockImpl.c
index 4eb42fb..c5fb176 100644
--- a/NetworkPkg/TcpDxe/SockImpl.c
+++ b/NetworkPkg/TcpDxe/SockImpl.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of the Socket.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -826,20 +826,12 @@ OnError:
 VOID
 SockDestroy (
   IN OUT SOCKET *Sock
   )
 {
-  VOID        *SockProtocol;
-  EFI_GUID    *TcpProtocolGuid;
-  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)) {
@@ -870,56 +862,10 @@ SockDestroy (
       );
 
     Sock->Parent = NULL;
   }
 
-  //
-  // Set the protocol guid and driver binding handle
-  // in the light of Sock->SockType
-  //
-  if (Sock->IpVersion == IP_VERSION_4) {
-    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;
-  } else {
-    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;
-  }
-
-  //
-  // Retrieve the protocol installed on this sock
-  //
-  Status = gBS->OpenProtocol (
-                  Sock->SockHandle,
-                  TcpProtocolGuid,
-                  &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,
-        TcpProtocolGuid,
-        SockProtocol,
-        NULL
-        );
-
-FreeSock:
-
   FreePool (Sock);
 }
 
 /**
   Flush the sndBuffer and rcvBuffer of socket.
diff --git a/NetworkPkg/TcpDxe/SockImpl.h b/NetworkPkg/TcpDxe/SockImpl.h
index 5a067de..80692b1 100644
--- a/NetworkPkg/TcpDxe/SockImpl.h
+++ b/NetworkPkg/TcpDxe/SockImpl.h
@@ -1,9 +1,9 @@
 /** @file
   The function declaration that provided for Socket Interface.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -15,10 +15,11 @@
 
 #ifndef _SOCK_IMPL_H_
 #define _SOCK_IMPL_H_
 
 #include "Socket.h"
+#include "TcpMain.h"
 
 /**
   Signal a event with the given status.
 
   @param[in] Token        The token's event is to be signaled.
diff --git a/NetworkPkg/TcpDxe/SockInterface.c b/NetworkPkg/TcpDxe/SockInterface.c
index 21ce643..b4ba40a 100644
--- a/NetworkPkg/TcpDxe/SockInterface.c
+++ b/NetworkPkg/TcpDxe/SockInterface.c
@@ -1,9 +1,9 @@
 /** @file
   Interface function of the Socket.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -140,20 +140,37 @@ SockBufferToken (
 EFI_STATUS
 SockDestroyChild (
   IN OUT SOCKET *Sock
   )
 {
-  EFI_STATUS  Status;
+  EFI_STATUS       Status;
+  TCP_PROTO_DATA   *ProtoData;
+  TCP_CB           *Tcb;
+  EFI_GUID         *IpProtocolGuid;
+  EFI_GUID         *TcpProtocolGuid;
+  VOID             *SockProtocol;
 
   ASSERT ((Sock != NULL) && (Sock->ProtoHandler != NULL));
 
   if (Sock->InDestroy) {
     return EFI_SUCCESS;
   }
 
   Sock->InDestroy = TRUE;
 
+  if (Sock->IpVersion == IP_VERSION_4) {
+    IpProtocolGuid = &gEfiIp4ProtocolGuid;
+    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;
+  } else {
+    IpProtocolGuid = &gEfiIp6ProtocolGuid;
+    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;
+  }
+  ProtoData = (TCP_PROTO_DATA *) Sock->ProtoReserved;
+  Tcb       = ProtoData->TcpPcb;
+
+  ASSERT (Tcb != NULL);
+
   Status            = EfiAcquireLockOrFail (&(Sock->Lock));
   if (EFI_ERROR (Status)) {
 
     DEBUG (
       (EFI_D_ERROR,
@@ -163,10 +180,55 @@ SockDestroyChild (
 
     return EFI_ACCESS_DENIED;
   }
 
   //
+  // Close the IP protocol.
+  //
+  gBS->CloseProtocol (
+         Tcb->IpInfo->ChildHandle,
+         IpProtocolGuid,
+         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,
+                  TcpProtocolGuid,
+                  &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
+  //
+  gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        TcpProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+
+  //
   // force protocol layer to detach the PCB
   //
   Status = Sock->ProtoHandler (Sock, SOCK_DETACH, NULL);
 
   if (EFI_ERROR (Status)) {
@@ -211,10 +273,12 @@ SockCreateChild (
   IN SOCK_INIT_DATA *SockInitData
   )
 {
   SOCKET      *Sock;
   EFI_STATUS  Status;
+  VOID        *SockProtocol;
+  EFI_GUID    *TcpProtocolGuid;
 
   //
   // create a new socket
   //
   Sock = SockCreate (SockInitData);
@@ -234,13 +298,11 @@ SockCreateChild (
     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
   //
@@ -251,16 +313,46 @@ SockCreateChild (
     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);
+  }
+
+  if (Sock->IpVersion == IP_VERSION_4) {
+    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;
+  } else {
+    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;
+  }
+
+  gBS->OpenProtocol (
+         Sock->SockHandle,
+         TcpProtocolGuid,
+         &SockProtocol,
+         Sock->DriverBinding,
+         Sock->SockHandle,
+         EFI_OPEN_PROTOCOL_GET_PROTOCOL
+         );
+  //
+  // Uninstall the protocol installed on this sock
+  //
+  gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        TcpProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+   SockDestroy (Sock);
+   return NULL;
 }
 
 /**
   Configure the specific socket Sock using configuration data ConfigData.
 
diff --git a/NetworkPkg/TcpDxe/TcpDispatcher.c b/NetworkPkg/TcpDxe/TcpDispatcher.c
index d4bc8ac..9a352b1 100644
--- a/NetworkPkg/TcpDxe/TcpDispatcher.c
+++ b/NetworkPkg/TcpDxe/TcpDispatcher.c
@@ -1,10 +1,10 @@
 /** @file
   The implementation of a dispatch routine for processing TCP requests.
 
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
-  Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -421,34 +421,17 @@ TcpDetachPcb (
   IN OUT SOCKET    *Sk
   )
 {
   TCP_PROTO_DATA   *ProtoData;
   TCP_CB           *Tcb;
-  EFI_GUID         *IpProtocolGuid;
 
-  if (Sk->IpVersion == IP_VERSION_4) {
-    IpProtocolGuid = &gEfiIp4ProtocolGuid;
-  } else {
-    IpProtocolGuid = &gEfiIp6ProtocolGuid;
-  }
-  
   ProtoData = (TCP_PROTO_DATA *) Sk->ProtoReserved;
   Tcb       = ProtoData->TcpPcb;
 
   ASSERT (Tcb != NULL);
 
   TcpFlushPcb (Tcb);
-
-  //
-  // Close the IP protocol.
-  //
-  gBS->CloseProtocol (
-         Tcb->IpInfo->ChildHandle,
-         IpProtocolGuid,
-         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] 4+ messages in thread

* Re: [PATCH V2] NetworkPkg: Fix service binding issue in TCP dxe.
  2017-03-15  2:27 [PATCH V2] NetworkPkg: Fix service binding issue in TCP dxe Zhang Lubo
@ 2017-03-16  1:46 ` Fu, Siyuan
  2017-03-16  3:50 ` Wu, Jiaxin
  2017-03-16  5:28 ` Ye, Ting
  2 siblings, 0 replies; 4+ 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] NetworkPkg: Fix service binding issue in TCP 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 perform the driverbing stop to abort tcp session and send the tcp reset packet, 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>
---
 NetworkPkg/TcpDxe/SockImpl.c      |  56 +-------------------
 NetworkPkg/TcpDxe/SockImpl.h      |   3 +-
 NetworkPkg/TcpDxe/SockInterface.c | 108 +++++++++++++++++++++++++++++++++++---
 NetworkPkg/TcpDxe/TcpDispatcher.c |  19 +------
 4 files changed, 104 insertions(+), 82 deletions(-)

diff --git a/NetworkPkg/TcpDxe/SockImpl.c b/NetworkPkg/TcpDxe/SockImpl.c index 4eb42fb..c5fb176 100644
--- a/NetworkPkg/TcpDxe/SockImpl.c
+++ b/NetworkPkg/TcpDxe/SockImpl.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of the Socket.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -826,20 +826,12 @@ OnError:
 VOID
 SockDestroy (
   IN OUT SOCKET *Sock
   )
 {
-  VOID        *SockProtocol;
-  EFI_GUID    *TcpProtocolGuid;
-  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)) {
@@ -870,56 +862,10 @@ SockDestroy (
       );
 
     Sock->Parent = NULL;
   }
 
-  //
-  // Set the protocol guid and driver binding handle
-  // in the light of Sock->SockType
-  //
-  if (Sock->IpVersion == IP_VERSION_4) {
-    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;
-  } else {
-    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;
-  }
-
-  //
-  // Retrieve the protocol installed on this sock
-  //
-  Status = gBS->OpenProtocol (
-                  Sock->SockHandle,
-                  TcpProtocolGuid,
-                  &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,
-        TcpProtocolGuid,
-        SockProtocol,
-        NULL
-        );
-
-FreeSock:
-
   FreePool (Sock);
 }
 
 /**
   Flush the sndBuffer and rcvBuffer of socket.
diff --git a/NetworkPkg/TcpDxe/SockImpl.h b/NetworkPkg/TcpDxe/SockImpl.h index 5a067de..80692b1 100644
--- a/NetworkPkg/TcpDxe/SockImpl.h
+++ b/NetworkPkg/TcpDxe/SockImpl.h
@@ -1,9 +1,9 @@
 /** @file
   The function declaration that provided for Socket Interface.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -15,10 +15,11 @@
 
 #ifndef _SOCK_IMPL_H_
 #define _SOCK_IMPL_H_
 
 #include "Socket.h"
+#include "TcpMain.h"
 
 /**
   Signal a event with the given status.
 
   @param[in] Token        The token's event is to be signaled.
diff --git a/NetworkPkg/TcpDxe/SockInterface.c b/NetworkPkg/TcpDxe/SockInterface.c
index 21ce643..b4ba40a 100644
--- a/NetworkPkg/TcpDxe/SockInterface.c
+++ b/NetworkPkg/TcpDxe/SockInterface.c
@@ -1,9 +1,9 @@
 /** @file
   Interface function of the Socket.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -140,20 +140,37 @@ SockBufferToken (
 EFI_STATUS
 SockDestroyChild (
   IN OUT SOCKET *Sock
   )
 {
-  EFI_STATUS  Status;
+  EFI_STATUS       Status;
+  TCP_PROTO_DATA   *ProtoData;
+  TCP_CB           *Tcb;
+  EFI_GUID         *IpProtocolGuid;
+  EFI_GUID         *TcpProtocolGuid;
+  VOID             *SockProtocol;
 
   ASSERT ((Sock != NULL) && (Sock->ProtoHandler != NULL));
 
   if (Sock->InDestroy) {
     return EFI_SUCCESS;
   }
 
   Sock->InDestroy = TRUE;
 
+  if (Sock->IpVersion == IP_VERSION_4) {
+    IpProtocolGuid = &gEfiIp4ProtocolGuid;
+    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;  } else {
+    IpProtocolGuid = &gEfiIp6ProtocolGuid;
+    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;  }  ProtoData = 
+ (TCP_PROTO_DATA *) Sock->ProtoReserved;
+  Tcb       = ProtoData->TcpPcb;
+
+  ASSERT (Tcb != NULL);
+
   Status            = EfiAcquireLockOrFail (&(Sock->Lock));
   if (EFI_ERROR (Status)) {
 
     DEBUG (
       (EFI_D_ERROR,
@@ -163,10 +180,55 @@ SockDestroyChild (
 
     return EFI_ACCESS_DENIED;
   }
 
   //
+  // Close the IP protocol.
+  //
+  gBS->CloseProtocol (
+         Tcb->IpInfo->ChildHandle,
+         IpProtocolGuid,
+         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,
+                  TcpProtocolGuid,
+                  &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  //  
+ gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        TcpProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+
+  //
   // force protocol layer to detach the PCB
   //
   Status = Sock->ProtoHandler (Sock, SOCK_DETACH, NULL);
 
   if (EFI_ERROR (Status)) {
@@ -211,10 +273,12 @@ SockCreateChild (
   IN SOCK_INIT_DATA *SockInitData
   )
 {
   SOCKET      *Sock;
   EFI_STATUS  Status;
+  VOID        *SockProtocol;
+  EFI_GUID    *TcpProtocolGuid;
 
   //
   // create a new socket
   //
   Sock = SockCreate (SockInitData);
@@ -234,13 +298,11 @@ SockCreateChild (
     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
   //
@@ -251,16 +313,46 @@ SockCreateChild (
     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);  }
+
+  if (Sock->IpVersion == IP_VERSION_4) {
+    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;  } else {
+    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;  }
+
+  gBS->OpenProtocol (
+         Sock->SockHandle,
+         TcpProtocolGuid,
+         &SockProtocol,
+         Sock->DriverBinding,
+         Sock->SockHandle,
+         EFI_OPEN_PROTOCOL_GET_PROTOCOL
+         );
+  //
+  // Uninstall the protocol installed on this sock  //  
+ gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        TcpProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+   SockDestroy (Sock);
+   return NULL;
 }
 
 /**
   Configure the specific socket Sock using configuration data ConfigData.
 
diff --git a/NetworkPkg/TcpDxe/TcpDispatcher.c b/NetworkPkg/TcpDxe/TcpDispatcher.c
index d4bc8ac..9a352b1 100644
--- a/NetworkPkg/TcpDxe/TcpDispatcher.c
+++ b/NetworkPkg/TcpDxe/TcpDispatcher.c
@@ -1,10 +1,10 @@
 /** @file
   The implementation of a dispatch routine for processing TCP requests.
 
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
-  Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -421,34 +421,17 @@ TcpDetachPcb (
   IN OUT SOCKET    *Sk
   )
 {
   TCP_PROTO_DATA   *ProtoData;
   TCP_CB           *Tcb;
-  EFI_GUID         *IpProtocolGuid;
 
-  if (Sk->IpVersion == IP_VERSION_4) {
-    IpProtocolGuid = &gEfiIp4ProtocolGuid;
-  } else {
-    IpProtocolGuid = &gEfiIp6ProtocolGuid;
-  }
-  
   ProtoData = (TCP_PROTO_DATA *) Sk->ProtoReserved;
   Tcb       = ProtoData->TcpPcb;
 
   ASSERT (Tcb != NULL);
 
   TcpFlushPcb (Tcb);
-
-  //
-  // Close the IP protocol.
-  //
-  gBS->CloseProtocol (
-         Tcb->IpInfo->ChildHandle,
-         IpProtocolGuid,
-         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] 4+ messages in thread

* Re: [PATCH V2] NetworkPkg: Fix service binding issue in TCP dxe.
  2017-03-15  2:27 [PATCH V2] NetworkPkg: Fix service binding issue in TCP dxe Zhang Lubo
  2017-03-16  1:46 ` Fu, Siyuan
@ 2017-03-16  3:50 ` Wu, Jiaxin
  2017-03-16  5:28 ` Ye, Ting
  2 siblings, 0 replies; 4+ 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

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] NetworkPkg: Fix service binding issue in TCP 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
> perform the driverbing stop to abort tcp session and send the tcp reset
> packet, 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>
> ---
>  NetworkPkg/TcpDxe/SockImpl.c      |  56 +-------------------
>  NetworkPkg/TcpDxe/SockImpl.h      |   3 +-
>  NetworkPkg/TcpDxe/SockInterface.c | 108
> +++++++++++++++++++++++++++++++++++---
>  NetworkPkg/TcpDxe/TcpDispatcher.c |  19 +------
>  4 files changed, 104 insertions(+), 82 deletions(-)
> 
> diff --git a/NetworkPkg/TcpDxe/SockImpl.c
> b/NetworkPkg/TcpDxe/SockImpl.c
> index 4eb42fb..c5fb176 100644
> --- a/NetworkPkg/TcpDxe/SockImpl.c
> +++ b/NetworkPkg/TcpDxe/SockImpl.c
> @@ -1,9 +1,9 @@
>  /** @file
>    Implementation of the Socket.
> 
> -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 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.
> @@ -826,20 +826,12 @@ OnError:
>  VOID
>  SockDestroy (
>    IN OUT SOCKET *Sock
>    )
>  {
> -  VOID        *SockProtocol;
> -  EFI_GUID    *TcpProtocolGuid;
> -  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)) {
> @@ -870,56 +862,10 @@ SockDestroy (
>        );
> 
>      Sock->Parent = NULL;
>    }
> 
> -  //
> -  // Set the protocol guid and driver binding handle
> -  // in the light of Sock->SockType
> -  //
> -  if (Sock->IpVersion == IP_VERSION_4) {
> -    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;
> -  } else {
> -    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;
> -  }
> -
> -  //
> -  // Retrieve the protocol installed on this sock
> -  //
> -  Status = gBS->OpenProtocol (
> -                  Sock->SockHandle,
> -                  TcpProtocolGuid,
> -                  &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,
> -        TcpProtocolGuid,
> -        SockProtocol,
> -        NULL
> -        );
> -
> -FreeSock:
> -
>    FreePool (Sock);
>  }
> 
>  /**
>    Flush the sndBuffer and rcvBuffer of socket.
> diff --git a/NetworkPkg/TcpDxe/SockImpl.h
> b/NetworkPkg/TcpDxe/SockImpl.h
> index 5a067de..80692b1 100644
> --- a/NetworkPkg/TcpDxe/SockImpl.h
> +++ b/NetworkPkg/TcpDxe/SockImpl.h
> @@ -1,9 +1,9 @@
>  /** @file
>    The function declaration that provided for Socket Interface.
> 
> -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 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.
> @@ -15,10 +15,11 @@
> 
>  #ifndef _SOCK_IMPL_H_
>  #define _SOCK_IMPL_H_
> 
>  #include "Socket.h"
> +#include "TcpMain.h"
> 
>  /**
>    Signal a event with the given status.
> 
>    @param[in] Token        The token's event is to be signaled.
> diff --git a/NetworkPkg/TcpDxe/SockInterface.c
> b/NetworkPkg/TcpDxe/SockInterface.c
> index 21ce643..b4ba40a 100644
> --- a/NetworkPkg/TcpDxe/SockInterface.c
> +++ b/NetworkPkg/TcpDxe/SockInterface.c
> @@ -1,9 +1,9 @@
>  /** @file
>    Interface function of the Socket.
> 
> -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 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.
> @@ -140,20 +140,37 @@ SockBufferToken (
>  EFI_STATUS
>  SockDestroyChild (
>    IN OUT SOCKET *Sock
>    )
>  {
> -  EFI_STATUS  Status;
> +  EFI_STATUS       Status;
> +  TCP_PROTO_DATA   *ProtoData;
> +  TCP_CB           *Tcb;
> +  EFI_GUID         *IpProtocolGuid;
> +  EFI_GUID         *TcpProtocolGuid;
> +  VOID             *SockProtocol;
> 
>    ASSERT ((Sock != NULL) && (Sock->ProtoHandler != NULL));
> 
>    if (Sock->InDestroy) {
>      return EFI_SUCCESS;
>    }
> 
>    Sock->InDestroy = TRUE;
> 
> +  if (Sock->IpVersion == IP_VERSION_4) {
> +    IpProtocolGuid = &gEfiIp4ProtocolGuid;
> +    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;
> +  } else {
> +    IpProtocolGuid = &gEfiIp6ProtocolGuid;
> +    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;
> +  }
> +  ProtoData = (TCP_PROTO_DATA *) Sock->ProtoReserved;
> +  Tcb       = ProtoData->TcpPcb;
> +
> +  ASSERT (Tcb != NULL);
> +
>    Status            = EfiAcquireLockOrFail (&(Sock->Lock));
>    if (EFI_ERROR (Status)) {
> 
>      DEBUG (
>        (EFI_D_ERROR,
> @@ -163,10 +180,55 @@ SockDestroyChild (
> 
>      return EFI_ACCESS_DENIED;
>    }
> 
>    //
> +  // Close the IP protocol.
> +  //
> +  gBS->CloseProtocol (
> +         Tcb->IpInfo->ChildHandle,
> +         IpProtocolGuid,
> +         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,
> +                  TcpProtocolGuid,
> +                  &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
> +  //
> +  gBS->UninstallMultipleProtocolInterfaces (
> +        Sock->SockHandle,
> +        TcpProtocolGuid,
> +        SockProtocol,
> +        NULL
> +        );
> +
> +  //
>    // force protocol layer to detach the PCB
>    //
>    Status = Sock->ProtoHandler (Sock, SOCK_DETACH, NULL);
> 
>    if (EFI_ERROR (Status)) {
> @@ -211,10 +273,12 @@ SockCreateChild (
>    IN SOCK_INIT_DATA *SockInitData
>    )
>  {
>    SOCKET      *Sock;
>    EFI_STATUS  Status;
> +  VOID        *SockProtocol;
> +  EFI_GUID    *TcpProtocolGuid;
> 
>    //
>    // create a new socket
>    //
>    Sock = SockCreate (SockInitData);
> @@ -234,13 +298,11 @@ SockCreateChild (
>      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
>    //
> @@ -251,16 +313,46 @@ SockCreateChild (
>      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);
> +  }
> +
> +  if (Sock->IpVersion == IP_VERSION_4) {
> +    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;
> +  } else {
> +    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;
> +  }
> +
> +  gBS->OpenProtocol (
> +         Sock->SockHandle,
> +         TcpProtocolGuid,
> +         &SockProtocol,
> +         Sock->DriverBinding,
> +         Sock->SockHandle,
> +         EFI_OPEN_PROTOCOL_GET_PROTOCOL
> +         );
> +  //
> +  // Uninstall the protocol installed on this sock
> +  //
> +  gBS->UninstallMultipleProtocolInterfaces (
> +        Sock->SockHandle,
> +        TcpProtocolGuid,
> +        SockProtocol,
> +        NULL
> +        );
> +   SockDestroy (Sock);
> +   return NULL;
>  }
> 
>  /**
>    Configure the specific socket Sock using configuration data ConfigData.
> 
> diff --git a/NetworkPkg/TcpDxe/TcpDispatcher.c
> b/NetworkPkg/TcpDxe/TcpDispatcher.c
> index d4bc8ac..9a352b1 100644
> --- a/NetworkPkg/TcpDxe/TcpDispatcher.c
> +++ b/NetworkPkg/TcpDxe/TcpDispatcher.c
> @@ -1,10 +1,10 @@
>  /** @file
>    The implementation of a dispatch routine for processing TCP requests.
> 
>    (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> -  Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 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.
> @@ -421,34 +421,17 @@ TcpDetachPcb (
>    IN OUT SOCKET    *Sk
>    )
>  {
>    TCP_PROTO_DATA   *ProtoData;
>    TCP_CB           *Tcb;
> -  EFI_GUID         *IpProtocolGuid;
> 
> -  if (Sk->IpVersion == IP_VERSION_4) {
> -    IpProtocolGuid = &gEfiIp4ProtocolGuid;
> -  } else {
> -    IpProtocolGuid = &gEfiIp6ProtocolGuid;
> -  }
> -
>    ProtoData = (TCP_PROTO_DATA *) Sk->ProtoReserved;
>    Tcb       = ProtoData->TcpPcb;
> 
>    ASSERT (Tcb != NULL);
> 
>    TcpFlushPcb (Tcb);
> -
> -  //
> -  // Close the IP protocol.
> -  //
> -  gBS->CloseProtocol (
> -         Tcb->IpInfo->ChildHandle,
> -         IpProtocolGuid,
> -         ProtoData->TcpService->IpIo->Image,
> -         Sk->SockHandle
> -         );
> 
>    IpIoRemoveIp (ProtoData->TcpService->IpIo, Tcb->IpInfo);
> 
>    FreePool (Tcb);
> 
> --
> 1.9.5.msysgit.1



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

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

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


-----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] NetworkPkg: Fix service binding issue in TCP 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 perform the driverbing stop to abort tcp session and send the tcp reset packet, 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>
---
 NetworkPkg/TcpDxe/SockImpl.c      |  56 +-------------------
 NetworkPkg/TcpDxe/SockImpl.h      |   3 +-
 NetworkPkg/TcpDxe/SockInterface.c | 108 +++++++++++++++++++++++++++++++++++---
 NetworkPkg/TcpDxe/TcpDispatcher.c |  19 +------
 4 files changed, 104 insertions(+), 82 deletions(-)

diff --git a/NetworkPkg/TcpDxe/SockImpl.c b/NetworkPkg/TcpDxe/SockImpl.c index 4eb42fb..c5fb176 100644
--- a/NetworkPkg/TcpDxe/SockImpl.c
+++ b/NetworkPkg/TcpDxe/SockImpl.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of the Socket.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -826,20 +826,12 @@ OnError:
 VOID
 SockDestroy (
   IN OUT SOCKET *Sock
   )
 {
-  VOID        *SockProtocol;
-  EFI_GUID    *TcpProtocolGuid;
-  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)) {
@@ -870,56 +862,10 @@ SockDestroy (
       );
 
     Sock->Parent = NULL;
   }
 
-  //
-  // Set the protocol guid and driver binding handle
-  // in the light of Sock->SockType
-  //
-  if (Sock->IpVersion == IP_VERSION_4) {
-    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;
-  } else {
-    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;
-  }
-
-  //
-  // Retrieve the protocol installed on this sock
-  //
-  Status = gBS->OpenProtocol (
-                  Sock->SockHandle,
-                  TcpProtocolGuid,
-                  &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,
-        TcpProtocolGuid,
-        SockProtocol,
-        NULL
-        );
-
-FreeSock:
-
   FreePool (Sock);
 }
 
 /**
   Flush the sndBuffer and rcvBuffer of socket.
diff --git a/NetworkPkg/TcpDxe/SockImpl.h b/NetworkPkg/TcpDxe/SockImpl.h index 5a067de..80692b1 100644
--- a/NetworkPkg/TcpDxe/SockImpl.h
+++ b/NetworkPkg/TcpDxe/SockImpl.h
@@ -1,9 +1,9 @@
 /** @file
   The function declaration that provided for Socket Interface.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -15,10 +15,11 @@
 
 #ifndef _SOCK_IMPL_H_
 #define _SOCK_IMPL_H_
 
 #include "Socket.h"
+#include "TcpMain.h"
 
 /**
   Signal a event with the given status.
 
   @param[in] Token        The token's event is to be signaled.
diff --git a/NetworkPkg/TcpDxe/SockInterface.c b/NetworkPkg/TcpDxe/SockInterface.c
index 21ce643..b4ba40a 100644
--- a/NetworkPkg/TcpDxe/SockInterface.c
+++ b/NetworkPkg/TcpDxe/SockInterface.c
@@ -1,9 +1,9 @@
 /** @file
   Interface function of the Socket.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -140,20 +140,37 @@ SockBufferToken (
 EFI_STATUS
 SockDestroyChild (
   IN OUT SOCKET *Sock
   )
 {
-  EFI_STATUS  Status;
+  EFI_STATUS       Status;
+  TCP_PROTO_DATA   *ProtoData;
+  TCP_CB           *Tcb;
+  EFI_GUID         *IpProtocolGuid;
+  EFI_GUID         *TcpProtocolGuid;
+  VOID             *SockProtocol;
 
   ASSERT ((Sock != NULL) && (Sock->ProtoHandler != NULL));
 
   if (Sock->InDestroy) {
     return EFI_SUCCESS;
   }
 
   Sock->InDestroy = TRUE;
 
+  if (Sock->IpVersion == IP_VERSION_4) {
+    IpProtocolGuid = &gEfiIp4ProtocolGuid;
+    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;  } else {
+    IpProtocolGuid = &gEfiIp6ProtocolGuid;
+    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;  }  ProtoData = 
+ (TCP_PROTO_DATA *) Sock->ProtoReserved;
+  Tcb       = ProtoData->TcpPcb;
+
+  ASSERT (Tcb != NULL);
+
   Status            = EfiAcquireLockOrFail (&(Sock->Lock));
   if (EFI_ERROR (Status)) {
 
     DEBUG (
       (EFI_D_ERROR,
@@ -163,10 +180,55 @@ SockDestroyChild (
 
     return EFI_ACCESS_DENIED;
   }
 
   //
+  // Close the IP protocol.
+  //
+  gBS->CloseProtocol (
+         Tcb->IpInfo->ChildHandle,
+         IpProtocolGuid,
+         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,
+                  TcpProtocolGuid,
+                  &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  //  
+ gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        TcpProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+
+  //
   // force protocol layer to detach the PCB
   //
   Status = Sock->ProtoHandler (Sock, SOCK_DETACH, NULL);
 
   if (EFI_ERROR (Status)) {
@@ -211,10 +273,12 @@ SockCreateChild (
   IN SOCK_INIT_DATA *SockInitData
   )
 {
   SOCKET      *Sock;
   EFI_STATUS  Status;
+  VOID        *SockProtocol;
+  EFI_GUID    *TcpProtocolGuid;
 
   //
   // create a new socket
   //
   Sock = SockCreate (SockInitData);
@@ -234,13 +298,11 @@ SockCreateChild (
     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
   //
@@ -251,16 +313,46 @@ SockCreateChild (
     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);  }
+
+  if (Sock->IpVersion == IP_VERSION_4) {
+    TcpProtocolGuid = &gEfiTcp4ProtocolGuid;  } else {
+    TcpProtocolGuid = &gEfiTcp6ProtocolGuid;  }
+
+  gBS->OpenProtocol (
+         Sock->SockHandle,
+         TcpProtocolGuid,
+         &SockProtocol,
+         Sock->DriverBinding,
+         Sock->SockHandle,
+         EFI_OPEN_PROTOCOL_GET_PROTOCOL
+         );
+  //
+  // Uninstall the protocol installed on this sock  //  
+ gBS->UninstallMultipleProtocolInterfaces (
+        Sock->SockHandle,
+        TcpProtocolGuid,
+        SockProtocol,
+        NULL
+        );
+   SockDestroy (Sock);
+   return NULL;
 }
 
 /**
   Configure the specific socket Sock using configuration data ConfigData.
 
diff --git a/NetworkPkg/TcpDxe/TcpDispatcher.c b/NetworkPkg/TcpDxe/TcpDispatcher.c
index d4bc8ac..9a352b1 100644
--- a/NetworkPkg/TcpDxe/TcpDispatcher.c
+++ b/NetworkPkg/TcpDxe/TcpDispatcher.c
@@ -1,10 +1,10 @@
 /** @file
   The implementation of a dispatch routine for processing TCP requests.
 
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
-  Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2009 - 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.
@@ -421,34 +421,17 @@ TcpDetachPcb (
   IN OUT SOCKET    *Sk
   )
 {
   TCP_PROTO_DATA   *ProtoData;
   TCP_CB           *Tcb;
-  EFI_GUID         *IpProtocolGuid;
 
-  if (Sk->IpVersion == IP_VERSION_4) {
-    IpProtocolGuid = &gEfiIp4ProtocolGuid;
-  } else {
-    IpProtocolGuid = &gEfiIp6ProtocolGuid;
-  }
-  
   ProtoData = (TCP_PROTO_DATA *) Sk->ProtoReserved;
   Tcb       = ProtoData->TcpPcb;
 
   ASSERT (Tcb != NULL);
 
   TcpFlushPcb (Tcb);
-
-  //
-  // Close the IP protocol.
-  //
-  gBS->CloseProtocol (
-         Tcb->IpInfo->ChildHandle,
-         IpProtocolGuid,
-         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] 4+ messages in thread

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

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

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