From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0A7B5802BD for ; Wed, 15 Mar 2017 22:36:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1489642568; x=1521178568; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=TYI6zum8YxTni/0uup+2uzL+J5tZZkcCtcf6vyCARaE=; b=xrp0Ivu7b9GPGxY0Tu0jxp5vFrDiLzWOxubmGXd4J55/3/9HoCY7LYpv yGEw7Q3fgT7dPcCiyuDFcL1Xh+NvCA==; Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Mar 2017 22:36:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,170,1486454400"; d="scan'208";a="76741482" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga005.fm.intel.com with ESMTP; 15 Mar 2017 22:36:07 -0700 Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 15 Mar 2017 22:36:07 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 15 Mar 2017 22:36:07 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX104.ccr.corp.intel.com ([10.239.4.70]) with mapi id 14.03.0248.002; Thu, 16 Mar 2017 13:36:05 +0800 From: "Zhang, Lubo" To: "Ye, Ting" , "edk2-devel@lists.01.org" CC: "Wu, Jiaxin" , "Fu, Siyuan" Thread-Topic: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe. Thread-Index: AQHSnTPFEvFS5jemD0WnnQimnTF4saGW8dkQgAAB1zA= Date: Thu, 16 Mar 2017 05:36:04 +0000 Message-ID: <7619447B08B8F74DA4FF2A813B79803B39B03B06@shsmsx102.ccr.corp.intel.com> References: <1489544850-35284-1-git-send-email-lubo.zhang@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH V2] MdeModulePkg: Fix service binding issue in TCP4 and Ip4 dxe. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Mar 2017 05:36:08 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Go it, thanks for your comments Best Regards Lubo=20 -----Original Message----- From: Ye, Ting=20 Sent: Thursday, March 16, 2017 1:31 PM To: Zhang, Lubo ; edk2-devel@lists.01.org Cc: Wu, Jiaxin ; Fu, Siyuan 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 poss= ible places, since the patch removed the value.=20 Also update CopyRight year in SockImpl.h before check-in. Others are good t= o me. Reviewed-by: Ye Ting Best Regards, Ting -----Original Message----- From: Zhang, Lubo=20 Sent: Wednesday, March 15, 2017 10:28 AM To: edk2-devel@lists.01.org Cc: Wu, Jiaxin ; Ye, Ting ; Fu, Siy= uan 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 c= hildHandle and last to free any data structures that allocated in CreateChi= ld. But currently, we free the socket data (Socket ConfigureState) before r= emoving 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 Cc: Wu Jiaxin Cc: Ye Ting Cc: Fu Siyuan --- 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 +----------- MdeM= odulePkg/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/MdeModuleP= kg/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. =20 -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
(C) Copyright 2015 Hewlett-Packard Development Company, L.P.
=20 This program and the accompanying materials are licensed and made availab= le under the terms and conditions of the BSD License which accompanies thi= s distribution. The full text of the license may be found at @@ -920,11 +9= 20,10 @@ Ip4ServiceBindingDestroyChild ( EFI_STATUS Status; IP4_SERVICE *IpSb; IP4_PROTOCOL *IpInstance; EFI_IP4_PROTOCOL *Ip4; EFI_TPL OldTpl; - INTN State; =20 if ((This =3D=3D NULL) || (ChildHandle =3D=3D NULL)) { return EFI_INVALID_PARAMETER; } =20 @@ -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 =3D=3D IP4_STATE_DESTROY) { + if (IpInstance->InDestroy) { gBS->RestoreTPL (OldTpl); return EFI_SUCCESS; } =20 - State =3D IpInstance->State; - IpInstance->State =3D IP4_STATE_DESTROY; + IpInstance->InDestroy =3D TRUE; =20 // // Close the Managed Network protocol. // gBS->CloseProtocol ( @@ -1007,10 +1005,11 @@ Ip4ServiceBindingDestroyChild ( &gEfiIp4ProtocolGuid, &IpInstance->Ip4Proto ); OldTpl =3D gBS->RaiseTPL (TPL_CALLBACK); if (EFI_ERROR (Status)) { + IpInstance->InDestroy =3D FALSE; goto ON_ERROR; } =20 Status =3D Ip4CleanProtocol (IpInstance); if (EFI_ERROR (Status)) { @@ -1031,10 +1030,9 @@ Ip4ServiceBindingDestroyChild ( =20 FreePool (IpInstance); return EFI_SUCCESS; =20 ON_ERROR: - IpInstance->State =3D State; gBS->RestoreTPL (OldTpl); =20 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 =20 -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made availab= le under the terms and conditions of the BSD License which accompanies thi= s distribution. The full text of the license may be found at http://opens= ource.org/licenses/bsd-license.php =20 @@ -548,10 +548,11 @@ Ip4InitProtocol ( ZeroMem (IpInstance, sizeof (IP4_PROTOCOL)); =20 IpInstance->Signature =3D IP4_PROTOCOL_SIGNATURE; CopyMem (&IpInstance->Ip4Proto, &mEfiIp4ProtocolTemplete, sizeof (IpInst= ance->Ip4Proto)); IpInstance->State =3D IP4_STATE_UNCONFIGED; + IpInstance->InDestroy =3D FALSE; IpInstance->Service =3D IpSb; =20 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. =20 -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
(C) Copyright 2015 Hewlett-Packard Development Company, L.P.
=20 This program and the accompanying materials are licensed and made availab= le under the terms and conditions of the BSD License which accompanies thi= s 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 // b= ecomes DESTROY. // #define IP4_STATE_UNCONFIGED 0 #define IP4_STATE_CONFIGED 1 -#define IP4_STATE_DESTROY 2 =20 // // 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 { =20 EFI_IP4_PROTOCOL Ip4Proto; EFI_HANDLE Handle; INTN State; =20 + BOOLEAN InDestroy; =20 + IP4_SERVICE *Service; LIST_ENTRY Link; // Link to all the IP protocol fro= m the service =20 // // User's transmit/receive tokens, and received/deliverd packets diff --= git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.c b/MdeModulePkg/Univ= ersal/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. =20 -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made availab= le under the terms and conditions of the BSD License which accompanies thi= s distribution. The full text of the license may be found at http://opens= ource.org/licenses/bsd-license.php
=20 @@ -715,20 +715,12 @@ OnError: VOID SockDestroy ( IN OUT SOCKET *Sock ) { - VOID *SockProtocol; - EFI_GUID *ProtocolGuid; - EFI_STATUS Status; - ASSERT (SockStream =3D=3D Sock->Type); =20 - if (Sock->DestroyCallback !=3D 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 ( ); =20 Sock->Parent =3D NULL; } =20 - // - // Set the protocol guid and driver binding handle - // in the light of Sock->SockType - // - ProtocolGuid =3D &gEfiTcp4ProtocolGuid; - - // - // Retrieve the protocol installed on this sock - // - Status =3D 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 ; } =20 =20 diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockImpl.h b/MdeModuleP= kg/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. =20 -Copyright (c) 2005 - 2006, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2007, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made availab= le under the terms and conditions of the BSD License which accompanies thi= s distribution. The full text of the license may be found at http://opens= ource.org/licenses/bsd-license.php
=20 @@ -14,10 +14,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITH= ER EXPRESS OR IMPLIED. =20 #ifndef _SOCK_IMPL_H_ #define _SOCK_IMPL_H_ =20 #include "Socket.h" +#include "Tcp4Main.h" =20 /** Signal a event with the given status. =20 @param Token The token's event is to be signaled. diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/SockInterface.c b/MdeMo= dulePkg/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. =20 -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made availab= le under the terms and conditions of the BSD License which accompanies thi= s distribution. The full text of the license may be found at http://opens= ource.org/licenses/bsd-license.php
=20 @@ -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; =20 ASSERT ((Sock !=3D NULL) && (Sock->ProtoHandler !=3D NULL)); =20 if (Sock->InDestroy) { return EFI_SUCCESS; } =20 Sock->InDestroy =3D TRUE; =20 + ProtoData =3D (TCP4_PROTO_DATA *) Sock->ProtoReserved; + Tcb =3D ProtoData->TcpPcb; + + ASSERT (Tcb !=3D NULL); + Status =3D EfiAcquireLockOrFail (&(Sock->Lock)); if (EFI_ERROR (Status)) { =20 DEBUG ((EFI_D_ERROR, "SockDestroyChild: Get the lock to " "access socket failed with %r\n", Status)); =20 return EFI_ACCESS_DENIED; } =20 // + // Close the IP protocol. + // + gBS->CloseProtocol ( + Tcb->IpInfo->ChildHandle, + &gEfiIp4ProtocolGuid, + ProtoData->TcpService->IpIo->Image, + Sock->SockHandle + ); + + if (Sock->DestroyCallback !=3D NULL) { + Sock->DestroyCallback (Sock, Sock->Context); } + + // + // Retrieve the protocol installed on this sock // Status =3D=20 + 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=20 + Sock->SockType // gBS->UninstallMultipleProtocolInterfaces ( + Sock->SockHandle, + &gEfiTcp4ProtocolGuid, + SockProtocol, + NULL + ); + + // // force protocol layer to detach the PCB // Status =3D Sock->ProtoHandler (Sock, SOCK_DETACH, NULL); =20 if (EFI_ERROR (Status)) { @@ -207,10 +258,11 @@ SOCKET * SockCreateChild ( IN SOCK_INIT_DATA *SockInitData ) { SOCKET *Sock; + VOID *SockProtocol; EFI_STATUS Status; =20 // // create a new socket // @@ -227,12 +279,11 @@ SockCreateChild ( if (EFI_ERROR (Status)) { =20 DEBUG ((EFI_D_ERROR, "SockCreateChild: Get the lock to " "access socket failed with %r\n", Status)); =20 - 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)) { =20 DEBUG ((EFI_D_ERROR, "SockCreateChild: Protocol failed to" " attach a socket with %r\n", Status)); =20 - SockDestroy (Sock); - Sock =3D NULL; + goto ERROR; } =20 return Sock; + +ERROR: + + if (Sock->DestroyCallback !=3D 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 // =20 + gBS->UninstallMultipleProtocolInterfaces ( + Sock->SockHandle, + &gEfiTcp4ProtocolGuid, + SockProtocol, + NULL + ); + SockDestroy (Sock); + return NULL; } =20 =20 /** Configure the specific socket Sock using configuration data ConfigData. diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dispatcher.c b/MdeM= odulePkg/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. =20 (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
-Copyrigh= t (c) 2005 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made availab= le under the terms and conditions of the BSD License which accompanies thi= s distribution. The full text of the license may be found at http://opens= ource.org/licenses/bsd-license.php
=20 @@ -330,20 +330,10 @@ Tcp4DetachPcb ( Tcb =3D ProtoData->TcpPcb; =20 ASSERT (Tcb !=3D NULL); =20 Tcp4FlushPcb (Tcb); - - // - // Close the IP protocol. - // - gBS->CloseProtocol ( - Tcb->IpInfo->ChildHandle, - &gEfiIp4ProtocolGuid, - ProtoData->TcpService->IpIo->Image, - Sk->SockHandle - ); =20 IpIoRemoveIp (ProtoData->TcpService->IpIo, Tcb->IpInfo); =20 FreePool (Tcb); =20 -- 1.9.5.msysgit.1