[Patch 0/2] Fix the bug when cleaning up the TLS instance

[Patch 0/2] Fix the bug when cleaning up the TLS instance

[Jiaxin Wu]

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Long Qin <qin.long@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>

Jiaxin Wu (2):
  CryptoPkg/TlsLib: Remove the redundant free of BIO objects
  NetworkPkg/HttpDxe: Destroy the TLS instance when cleaning up the HTTP child

 CryptoPkg/Library/TlsLib/TlsInit.c | 10 +---------
 NetworkPkg/HttpDxe/HttpImpl.c      |  1 +
 NetworkPkg/HttpDxe/HttpProto.c     |  7 +++++++
 NetworkPkg/HttpDxe/HttpProto.h     |  3 ++-
 NetworkPkg/HttpDxe/HttpsSupport.c  | 14 +++++++-------
 NetworkPkg/HttpDxe/HttpsSupport.h  |  4 +++-
 6 files changed, 21 insertions(+), 18 deletions(-)

-- 
1.9.5.msysgit.1

[Patch 1/2] CryptoPkg/TlsLib: Remove the redundant free of BIO objects

[Jiaxin Wu]

TLS BIO objects (InBio/OutBio) will be freed by SSL_free() function.
So, the following free operation (BIO_free) in TlsFree is redundant.
It can be removed directly.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Long Qin <qin.long@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 CryptoPkg/Library/TlsLib/TlsInit.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/CryptoPkg/Library/TlsLib/TlsInit.c b/CryptoPkg/Library/TlsLib/TlsInit.c
index e2c9744..e524647 100644
--- a/CryptoPkg/Library/TlsLib/TlsInit.c
+++ b/CryptoPkg/Library/TlsLib/TlsInit.c
@@ -128,24 +128,16 @@ TlsFree (
   if (TlsConn == NULL) {
     return;
   }
 
   //
-  // Free the internal TLS and BIO objects.
+  // Free the internal TLS and related BIO objects.
   //
   if (TlsConn->Ssl != NULL) {
     SSL_free (TlsConn->Ssl);
   }
 
-  if (TlsConn->InBio != NULL) {
-    BIO_free (TlsConn->InBio);
-  }
-
-  if (TlsConn->OutBio != NULL) {
-    BIO_free (TlsConn->OutBio);
-  }
-
   OPENSSL_free (Tls);
 }
 
 /**
   Create a new TLS object for a connection.
-- 
1.9.5.msysgit.1

[Patch 2/2] NetworkPkg/HttpDxe: Destroy the TLS instance when cleaning up the HTTP child

[Jiaxin Wu]

During clean up the HTTP child, all resources used by it should be cleaned. But
currently, TLS instance is not destroyed.

This patch is to fix this issue.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/HttpDxe/HttpImpl.c     |  1 +
 NetworkPkg/HttpDxe/HttpProto.c    |  7 +++++++
 NetworkPkg/HttpDxe/HttpProto.h    |  3 ++-
 NetworkPkg/HttpDxe/HttpsSupport.c | 14 +++++++-------
 NetworkPkg/HttpDxe/HttpsSupport.h |  4 +++-
 5 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 1f7a4fa..9570053 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -378,10 +378,11 @@ EfiHttpRequest (
         ImageHandle = HttpInstance->Service->Ip4DriverBindingHandle;
       }
 
       HttpInstance->TlsChildHandle = TlsCreateChild (
                                        ImageHandle,
+                                       &(HttpInstance->TlsSb),
                                        &(HttpInstance->Tls),
                                        &(HttpInstance->TlsConfiguration)
                                        );
       if (HttpInstance->TlsChildHandle == NULL) {
         return EFI_DEVICE_ERROR;
diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
index 3fda294..ab00f3d 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -862,10 +862,17 @@ HttpCleanProtocol (
   }
   
   NetMapClean (&HttpInstance->TxTokens);
   NetMapClean (&HttpInstance->RxTokens);
 
+  if (HttpInstance->TlsSb != NULL && HttpInstance->TlsChildHandle != NULL) {
+    //
+    // Destroy the TLS instance.   
+    //
+    HttpInstance->TlsSb->DestroyChild (HttpInstance->TlsSb, HttpInstance->TlsChildHandle);
+  }
+
   if (HttpInstance->Tcp4ChildHandle != NULL) {
     gBS->CloseProtocol (
            HttpInstance->Tcp4ChildHandle,
            &gEfiTcp4ProtocolGuid,
            HttpInstance->Service->Ip4DriverBindingHandle,
diff --git a/NetworkPkg/HttpDxe/HttpProto.h b/NetworkPkg/HttpDxe/HttpProto.h
index 95fb484..04d36aa 100644
--- a/NetworkPkg/HttpDxe/HttpProto.h
+++ b/NetworkPkg/HttpDxe/HttpProto.h
@@ -164,11 +164,12 @@ typedef struct _HTTP_PROTOCOL {
 
   //
   // Https Support
   //
   BOOLEAN                          UseHttps;
-  
+
+  EFI_SERVICE_BINDING_PROTOCOL     *TlsSb;
   EFI_HANDLE                       TlsChildHandle; /// Tls ChildHandle
   TLS_CONFIG_DATA                  TlsConfigData;
   EFI_TLS_PROTOCOL                 *Tls;
   EFI_TLS_CONFIGURATION_PROTOCOL   *TlsConfiguration;
   EFI_TLS_SESSION_STATE            TlsSessionState;
diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
index e4d9a37..e6f4d5a 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -138,44 +138,44 @@ IsHttpsUrl (
 
 /**
   Creates a Tls child handle, open EFI_TLS_PROTOCOL and EFI_TLS_CONFIGURATION_PROTOCOL.
 
   @param[in]  ImageHandle           The firmware allocated handle for the UEFI image.
+  @param[out] TlsSb                 Pointer to the TLS SERVICE_BINDING_PROTOCOL.
   @param[out] TlsProto              Pointer to the EFI_TLS_PROTOCOL instance.
   @param[out] TlsConfiguration      Pointer to the EFI_TLS_CONFIGURATION_PROTOCOL instance.
 
   @return  The child handle with opened EFI_TLS_PROTOCOL and EFI_TLS_CONFIGURATION_PROTOCOL.
 
 **/
 EFI_HANDLE
 EFIAPI
 TlsCreateChild (
   IN  EFI_HANDLE                     ImageHandle,
+  OUT EFI_SERVICE_BINDING_PROTOCOL   **TlsSb,
   OUT EFI_TLS_PROTOCOL               **TlsProto,
   OUT EFI_TLS_CONFIGURATION_PROTOCOL **TlsConfiguration
   )
 {
   EFI_STATUS                    Status;
-  EFI_SERVICE_BINDING_PROTOCOL  *TlsSb;
   EFI_HANDLE                    TlsChildHandle;
 
-  TlsSb          = NULL;
   TlsChildHandle = 0;
 
   //
   // Locate TlsServiceBinding protocol.
   //
   gBS->LocateProtocol (
      &gEfiTlsServiceBindingProtocolGuid,
      NULL,
-     (VOID **) &TlsSb
+     (VOID **) TlsSb
      );
-  if (TlsSb == NULL) {
+  if (*TlsSb == NULL) {
     return NULL;
   }
 
-  Status = TlsSb->CreateChild (TlsSb, &TlsChildHandle);
+  Status = (*TlsSb)->CreateChild (*TlsSb, &TlsChildHandle);
   if (EFI_ERROR (Status)) {
     return NULL;
   }
 
   Status = gBS->OpenProtocol (
@@ -185,11 +185,11 @@ TlsCreateChild (
                   ImageHandle,
                   TlsChildHandle,
                   EFI_OPEN_PROTOCOL_GET_PROTOCOL
                   );
   if (EFI_ERROR (Status)) {
-    TlsSb->DestroyChild (TlsSb, TlsChildHandle);
+    (*TlsSb)->DestroyChild (*TlsSb, TlsChildHandle);
     return NULL;
   }
 
   Status = gBS->OpenProtocol (
                   TlsChildHandle,
@@ -198,11 +198,11 @@ TlsCreateChild (
                   ImageHandle,
                   TlsChildHandle,
                   EFI_OPEN_PROTOCOL_GET_PROTOCOL
                   );
   if (EFI_ERROR (Status)) {
-    TlsSb->DestroyChild (TlsSb, TlsChildHandle);
+    (*TlsSb)->DestroyChild (*TlsSb, TlsChildHandle);
     return NULL;
   }
 
   return TlsChildHandle;
 }
diff --git a/NetworkPkg/HttpDxe/HttpsSupport.h b/NetworkPkg/HttpDxe/HttpsSupport.h
index 68a6073..f7a2d30 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.h
+++ b/NetworkPkg/HttpDxe/HttpsSupport.h
@@ -1,9 +1,9 @@
 /** @file
   The header files of miscellaneous routines specific to Https for HttpDxe driver.
 
-Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 - 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
 
@@ -35,20 +35,22 @@ IsHttpsUrl (
 
 /**
   Creates a Tls child handle, open EFI_TLS_PROTOCOL and EFI_TLS_CONFIGURATION_PROTOCOL.
 
   @param[in]  ImageHandle           The firmware allocated handle for the UEFI image.
+  @param[out] TlsSb                 Pointer to the TLS SERVICE_BINDING_PROTOCOL.
   @param[out] TlsProto              Pointer to the EFI_TLS_PROTOCOL instance.
   @param[out] TlsConfiguration      Pointer to the EFI_TLS_CONFIGURATION_PROTOCOL instance.
 
   @return  The child handle with opened EFI_TLS_PROTOCOL and EFI_TLS_CONFIGURATION_PROTOCOL.
 
 **/
 EFI_HANDLE
 EFIAPI
 TlsCreateChild (
   IN  EFI_HANDLE                     ImageHandle,
+  OUT EFI_SERVICE_BINDING_PROTOCOL   **TlsSb,
   OUT EFI_TLS_PROTOCOL               **TlsProto,
   OUT EFI_TLS_CONFIGURATION_PROTOCOL **TlsConfiguration
   );
 
 /**
-- 
1.9.5.msysgit.1

Re: [Patch 1/2] CryptoPkg/TlsLib: Remove the redundant free of BIO objects

[Long, Qin]

Reviewed-by: Long Qin <qin.long@intel.com>


-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiaxin Wu
Sent: Monday, July 31, 2017 1:41 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Long, Qin <qin.long@intel.com>
Subject: [edk2] [Patch 1/2] CryptoPkg/TlsLib: Remove the redundant free of BIO objects

TLS BIO objects (InBio/OutBio) will be freed by SSL_free() function.
So, the following free operation (BIO_free) in TlsFree is redundant.
It can be removed directly.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Long Qin <qin.long@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 CryptoPkg/Library/TlsLib/TlsInit.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/CryptoPkg/Library/TlsLib/TlsInit.c b/CryptoPkg/Library/TlsLib/TlsInit.c
index e2c9744..e524647 100644
--- a/CryptoPkg/Library/TlsLib/TlsInit.c
+++ b/CryptoPkg/Library/TlsLib/TlsInit.c
@@ -128,24 +128,16 @@ TlsFree (
   if (TlsConn == NULL) {
     return;
   }
 
   //
-  // Free the internal TLS and BIO objects.
+  // Free the internal TLS and related BIO objects.
   //
   if (TlsConn->Ssl != NULL) {
     SSL_free (TlsConn->Ssl);
   }
 
-  if (TlsConn->InBio != NULL) {
-    BIO_free (TlsConn->InBio);
-  }
-
-  if (TlsConn->OutBio != NULL) {
-    BIO_free (TlsConn->OutBio);
-  }
-
   OPENSSL_free (Tls);
 }
 
 /**
   Create a new TLS object for a connection.
-- 
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [Patch 0/2] Fix the bug when cleaning up the TLS instance

[Fu, Siyuan]

Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>


-----Original Message-----
From: Wu, Jiaxin 
Sent: Monday, July 31, 2017 1:41 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Long, Qin <qin.long@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [Patch 0/2] Fix the bug when cleaning up the TLS instance

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Long Qin <qin.long@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>

Jiaxin Wu (2):
  CryptoPkg/TlsLib: Remove the redundant free of BIO objects
  NetworkPkg/HttpDxe: Destroy the TLS instance when cleaning up the HTTP child

 CryptoPkg/Library/TlsLib/TlsInit.c | 10 +---------
 NetworkPkg/HttpDxe/HttpImpl.c      |  1 +
 NetworkPkg/HttpDxe/HttpProto.c     |  7 +++++++
 NetworkPkg/HttpDxe/HttpProto.h     |  3 ++-
 NetworkPkg/HttpDxe/HttpsSupport.c  | 14 +++++++-------
 NetworkPkg/HttpDxe/HttpsSupport.h  |  4 +++-
 6 files changed, 21 insertions(+), 18 deletions(-)

-- 
1.9.5.msysgit.1