From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (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 DFF11803FA for ; Wed, 15 Mar 2017 20:50:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1489636232; x=1521172232; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=ujo0OwLE+0TWcYQU2nHvQu6SwlxTg7KlMioLZU1FALU=; b=W4sLdE6+FEDfNssd4sSZoqN1xucNVnCEppzJRxWBRZ8lDW8CLED1flH+ D+Esym8wAnNsBcu8W5CHEt71gewjmg==; Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Mar 2017 20:50:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,170,1486454400"; d="scan'208";a="75958057" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga005.jf.intel.com with ESMTP; 15 Mar 2017 20:50:29 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 15 Mar 2017 20:50:25 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.20]) by shsmsx102.ccr.corp.intel.com ([169.254.2.88]) with mapi id 14.03.0248.002; Thu, 16 Mar 2017 11:50:21 +0800 From: "Wu, Jiaxin" To: "Zhang, Lubo" , "edk2-devel@lists.01.org" CC: "Ye, Ting" , "Fu, Siyuan" Thread-Topic: [PATCH V2] NetworkPkg: Fix service binding issue in TCP dxe. Thread-Index: AQHSnTPPdtyVAIiFUku048sN/i6ahaGW1maw Date: Thu, 16 Mar 2017 03:50:21 +0000 Message-ID: <895558F6EA4E3B41AC93A00D163B7274162A3941@SHSMSX103.ccr.corp.intel.com> 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: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYmQ1NTE0ODktMDFkNy00YjY0LWFjM2EtZTAxOTU3ZWY2MzA2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IlQ1bEV5NE9EUVdSUFZjK0VuOGp6VVdRQXA5czVZM1QxUk9VSDlMV1hvaXM9In0= x-ctpclassification: CTP_IC 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 03:50:31 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Wu Jiaxin Thanks, Jiaxin > -----Original Message----- > From: Zhang, Lubo > Sent: Wednesday, March 15, 2017 10:28 AM > To: edk2-devel@lists.01.org > Cc: Wu, Jiaxin ; Ye, Ting ; Fu, > Siyuan > Subject: [PATCH V2] NetworkPkg: Fix service binding issue in TCP dxe. >=20 > v2: Handle error case in SockCreateChild and fix typo issue >=20 > 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 ConfigureStat= e) > 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. >=20 > 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(-) >=20 > 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. >=20 > - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
>=20 > 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 =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 > 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. >=20 > - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
>=20 > 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 @@ >=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/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. >=20 > - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
>=20 > 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; >=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 (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 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 =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 > + // > + 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/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. >=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 reserved.
>=20 > 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; >=20 > - if (Sk->IpVersion =3D=3D IP_VERSION_4) { > - IpProtocolGuid =3D &gEfiIp4ProtocolGuid; > - } else { > - IpProtocolGuid =3D &gEfiIp6ProtocolGuid; > - } > - > 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