From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (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 B9F4E802B0 for ; Wed, 15 Mar 2017 22:28:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1489642100; x=1521178100; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=RwrDYMW72sFx5OOGurmuSkTDkP21nD23kSdlbCUetQ4=; b=I09TcSHtOcQDI+8lM1tCnTXGpYoAveWEI0VAeDMLKk6YIEOMhQE39Kjg AhCxQEqqdSmb/WHcH1zHh8jcrLmmvQ==; Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Mar 2017 22:28:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,170,1486454400"; d="scan'208";a="944862694" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga003.jf.intel.com with ESMTP; 15 Mar 2017 22:28:20 -0700 Received: from fmsmsx119.amr.corp.intel.com (10.18.124.207) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 15 Mar 2017 22:28:19 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX119.amr.corp.intel.com (10.18.124.207) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 15 Mar 2017 22:28:19 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.20]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.132]) with mapi id 14.03.0248.002; Thu, 16 Mar 2017 13:28:17 +0800 From: "Ye, Ting" To: "Zhang, Lubo" , "edk2-devel@lists.01.org" CC: "Wu, Jiaxin" , "Fu, Siyuan" Thread-Topic: [PATCH V2] NetworkPkg: Fix service binding issue in TCP dxe. Thread-Index: AQHSnTPPtoFlEN1PAUCc9ZFF3IvVhKGW8Zrg Date: Thu, 16 Mar 2017 05:28:16 +0000 Message-ID: References: <1489544879-24940-1-git-send-email-lubo.zhang@intel.com> In-Reply-To: <1489544879-24940-1-git-send-email-lubo.zhang@intel.com> Accept-Language: zh-CN, 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] NetworkPkg: Fix service binding issue in TCP 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:28:21 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Ye Ting =20 -----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] 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 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 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 Cc: Wu Jiaxin Cc: Ye Ting Cc: Fu Siyuan --- 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 in= dex 4eb42fb..c5fb176 100644 --- a/NetworkPkg/TcpDxe/SockImpl.c +++ b/NetworkPkg/TcpDxe/SockImpl.c @@ -1,9 +1,9 @@ /** @file Implementation of the Socket. =20 - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2009 - 2017, Intel Corporation. All rights=20 + reserved.
=20 This program and the accompanying materials are licensed and made available under the terms and conditions of the BS= D License which accompanies this distribution. The full text of the license may b= e 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 =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)) { @@ -870,56 +862,10 @@ SockDestroy ( ); =20 Sock->Parent =3D NULL; } =20 - // - // Set the protocol guid and driver binding handle - // in the light of Sock->SockType - // - if (Sock->IpVersion =3D=3D IP_VERSION_4) { - TcpProtocolGuid =3D &gEfiTcp4ProtocolGuid; - } else { - TcpProtocolGuid =3D &gEfiTcp6ProtocolGuid; - } - - // - // Retrieve the protocol installed on this sock - // - Status =3D 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); } =20 /** Flush the sndBuffer and rcvBuffer of socket. diff --git a/NetworkPkg/TcpDxe/SockImpl.h b/NetworkPkg/TcpDxe/SockImpl.h in= dex 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. =20 - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2009 - 2017, Intel Corporation. All rights=20 + reserved.
=20 This program and the accompanying materials are licensed and made available under the terms and conditions of the BS= D License which accompanies this distribution. The full text of the license may b= e found at http://opensource.org/licenses/bsd-license.php. @@ -15,10 +15,11 @@ =20 #ifndef _SOCK_IMPL_H_ #define _SOCK_IMPL_H_ =20 #include "Socket.h" +#include "TcpMain.h" =20 /** Signal a event with the given status. =20 @param[in] Token The token's event is to be signaled. diff --git a/NetworkPkg/TcpDxe/SockInterface.c b/NetworkPkg/TcpDxe/SockInte= rface.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. =20 - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2009 - 2017, Intel Corporation. All rights=20 + reserved.
=20 This program and the accompanying materials are licensed and made available under the terms and conditions of the BS= D License which accompanies this distribution. The full text of the license may b= e 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; =20 ASSERT ((Sock !=3D NULL) && (Sock->ProtoHandler !=3D NULL)); =20 if (Sock->InDestroy) { return EFI_SUCCESS; } =20 Sock->InDestroy =3D TRUE; =20 + if (Sock->IpVersion =3D=3D IP_VERSION_4) { + IpProtocolGuid =3D &gEfiIp4ProtocolGuid; + TcpProtocolGuid =3D &gEfiTcp4ProtocolGuid; } else { + IpProtocolGuid =3D &gEfiIp6ProtocolGuid; + TcpProtocolGuid =3D &gEfiTcp6ProtocolGuid; } ProtoData =3D=20 + (TCP_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, @@ -163,10 +180,55 @@ SockDestroyChild ( =20 return EFI_ACCESS_DENIED; } =20 // + // Close the IP protocol. + // + gBS->CloseProtocol ( + Tcb->IpInfo->ChildHandle, + IpProtocolGuid, + 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, + 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 // =20 + gBS->UninstallMultipleProtocolInterfaces ( + Sock->SockHandle, + TcpProtocolGuid, + SockProtocol, + NULL + ); + + // // force protocol layer to detach the PCB // Status =3D Sock->ProtoHandler (Sock, SOCK_DETACH, NULL); =20 if (EFI_ERROR (Status)) { @@ -211,10 +273,12 @@ SockCreateChild ( IN SOCK_INIT_DATA *SockInitData ) { SOCKET *Sock; EFI_STATUS Status; + VOID *SockProtocol; + EFI_GUID *TcpProtocolGuid; =20 // // create a new socket // Sock =3D 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 =3D NULL; + goto ERROR; } =20 return Sock; + +ERROR: + + if (Sock->DestroyCallback !=3D NULL) { + Sock->DestroyCallback (Sock, Sock->Context); } + + if (Sock->IpVersion =3D=3D IP_VERSION_4) { + TcpProtocolGuid =3D &gEfiTcp4ProtocolGuid; } else { + TcpProtocolGuid =3D &gEfiTcp6ProtocolGuid; } + + gBS->OpenProtocol ( + Sock->SockHandle, + TcpProtocolGuid, + &SockProtocol, + Sock->DriverBinding, + Sock->SockHandle, + EFI_OPEN_PROTOCOL_GET_PROTOCOL + ); + // + // Uninstall the protocol installed on this sock // =20 + gBS->UninstallMultipleProtocolInterfaces ( + Sock->SockHandle, + TcpProtocolGuid, + SockProtocol, + NULL + ); + SockDestroy (Sock); + return NULL; } =20 /** Configure the specific socket Sock using configuration data ConfigData. =20 diff --git a/NetworkPkg/TcpDxe/TcpDispatcher.c b/NetworkPkg/TcpDxe/TcpDispa= tcher.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. =20 (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
- Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
+ Copyright (c) 2009 - 2017, Intel Corporation. All rights=20 + reserved.
=20 This program and the accompanying materials are licensed and made available under the terms and conditions of the BS= D License which accompanies this distribution. The full text of the license may b= e 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; =20 - if (Sk->IpVersion =3D=3D IP_VERSION_4) { - IpProtocolGuid =3D &gEfiIp4ProtocolGuid; - } else { - IpProtocolGuid =3D &gEfiIp6ProtocolGuid; - } - =20 ProtoData =3D (TCP_PROTO_DATA *) Sk->ProtoReserved; Tcb =3D ProtoData->TcpPcb; =20 ASSERT (Tcb !=3D NULL); =20 TcpFlushPcb (Tcb); - - // - // Close the IP protocol. - // - gBS->CloseProtocol ( - Tcb->IpInfo->ChildHandle, - IpProtocolGuid, - ProtoData->TcpService->IpIo->Image, - Sk->SockHandle - ); =20 IpIoRemoveIp (ProtoData->TcpService->IpIo, Tcb->IpInfo); =20 FreePool (Tcb); =20 -- 1.9.5.msysgit.1