subject:"\[edk2\] \[Patch\] NetworkPkg\: Avoid potential NULL pointer dereference"

Re: [edk2] [Patch] NetworkPkg: Avoid potential NULL pointer dereference

2016-06-24 Thread Wu, Jiaxin


> -Original Message-
> From: Ye, Ting
> Sent: Friday, June 24, 2016 4:25 PM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Zhang, Lubo 
> Subject: RE: [Patch] NetworkPkg: Avoid potential NULL pointer dereference
> 
> Looks good to me. I only have one small comment:
> 
> +  if (EFI_ERROR (Ikev2EncodePacket ((IKEV2_SESSION_COMMON *)
> SessionCommon, IkePacket, IkeType))) {
> +return NULL;
> +  }

Ting,
Do you mean any compilers will meet failure or warnings with this one line code?



> 
> Do we need divide it into two code lines to satisfy some compilers?
> Status = Ikev2EncodePacket ((IKEV2_SESSION_COMMON *)
> SessionCommon, IkePacket, IkeType)); if (EFI_ERROR (Status)) ...
> 
> As long as our supported compilers will not report build errors or warnings, I
> am fine with the update.
> 
> Reviewed-by: Ye Ting 
> 
> 
> 
> -Original Message-
> From: Wu, Jiaxin
> Sent: Friday, June 24, 2016 3:33 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Zhang,
> Lubo 
> Subject: [Patch] NetworkPkg: Avoid potential NULL pointer dereference
> 
> The commit of 6b16c9e7 removes ASSERT and use error handling in IpSecDxe
> driver, but may cause the potential NULL pointer dereference. So, this patch
> is used to avoid NULL pointer dereference.
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Zhang Lubo 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu 
> ---
>  NetworkPkg/IpSecDxe/IkePacket.c  |   6 +-
>  NetworkPkg/IpSecDxe/Ikev2/ChildSa.c  |  19 ++--
> NetworkPkg/IpSecDxe/Ikev2/Exchange.c |  10 ++-
>  NetworkPkg/IpSecDxe/Ikev2/Payload.c  |   3 +
>  NetworkPkg/IpSecDxe/Ikev2/Sa.c   | 163
> ++-
>  5 files changed, 188 insertions(+), 13 deletions(-)
> 
> diff --git a/NetworkPkg/IpSecDxe/IkePacket.c
> b/NetworkPkg/IpSecDxe/IkePacket.c index 8fd395d..d5a938e 100644
> --- a/NetworkPkg/IpSecDxe/IkePacket.c
> +++ b/NetworkPkg/IpSecDxe/IkePacket.c
> @@ -1,9 +1,9 @@
>  /** @file
>IKE Packet related operation.
> 
> -  Copyright (c) 2010, Intel Corporation. All rights reserved.
> +  Copyright (c) 2010 - 2016, Intel Corporation. All rights
> + reserved.
> 
>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.
> @@ -201,11 +201,13 @@ IkeNetbufFromPacket (
>  //
>  // Convert Host order to Network order for IKE_PACKET header and
> payloads
>  // Encryption payloads if needed
>  //
>  if (((IKEV2_SESSION_COMMON *) SessionCommon)->IkeVer == 2) {
> -  Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) SessionCommon,
> IkePacket, IkeType);
> +  if (EFI_ERROR (Ikev2EncodePacket ((IKEV2_SESSION_COMMON *)
> SessionCommon, IkePacket, IkeType))) {
> +return NULL;
> +  }
>  } else {
>//
>//If IKEv1 support, check it here.
>//
>return NULL;
> diff --git a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
> b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
> index d3859e2..1f0199b 100644
> --- a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
> +++ b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
> @@ -1,9 +1,9 @@
>  /** @file
>The operations for Child SA.
> 
> -  Copyright (c) 2010, Intel Corporation. All rights reserved.
> +  Copyright (c) 2010 - 2016, Intel Corporation. All rights
> + reserved.
> 
>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.
> @@ -37,22 +37,25 @@ Ikev2CreateChildGenerator (
>IKEV2_CHILD_SA_SESSION  *ChildSaSession;
>IKEV2_SA_SESSION*IkeSaSession;
>IKE_PACKET  *IkePacket;
>IKE_PAYLOAD *NotifyPayload;
>UINT32  *MessageId;
> +
> +  NotifyPayload   = NULL;
> +  MessageId   = NULL;
> 
>ChildSaSession  = (IKEV2_CHILD_SA_SESSION *) SaSession;
> -  IkePacket   = IkePacketAlloc();
> -  MessageId   = NULL;
> -
> -  if (IkePacket == NULL) {
> +  if (ChildSaSession == NULL) {
>  return NULL;
>}
> -  if (ChildSaSession == NULL) {
> +
> +  IkePacket   = IkePacketAlloc();
> +  if (IkePacket == NULL) {
>  return NULL;
>}
> 
> +
>if (Context != NULL) {
>  MessageId = (UINT32 *) Context;
>}
> 
>IkePacket->Header->Version  = (UINT8) (2 << 4);
> @@ -111,10 +114,14 @@ Ikev2CreateChildGenerator (
>  IKEV2_NOTIFICATION_NO_ADDITIONAL_SAS,
>  NULL,
>

Re: [edk2] [Patch] NetworkPkg: Avoid potential NULL pointer dereference

2016-06-24 Thread Ye, Ting

Looks good to me. I only have one small comment:

+  if (EFI_ERROR (Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) 
SessionCommon, IkePacket, IkeType))) {
+return NULL;
+  }

Do we need divide it into two code lines to satisfy some compilers? 
Status = Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) SessionCommon, IkePacket, 
IkeType));
if (EFI_ERROR (Status)) 
...

As long as our supported compilers will not report build errors or warnings, I 
am fine with the update.

Reviewed-by: Ye Ting 



-Original Message-
From: Wu, Jiaxin 
Sent: Friday, June 24, 2016 3:33 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan ; Zhang, Lubo 

Subject: [Patch] NetworkPkg: Avoid potential NULL pointer dereference

The commit of 6b16c9e7 removes ASSERT and use error handling in IpSecDxe 
driver, but may cause the potential NULL pointer dereference. So, this patch is 
used to avoid NULL pointer dereference.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/IpSecDxe/IkePacket.c  |   6 +-
 NetworkPkg/IpSecDxe/Ikev2/ChildSa.c  |  19 ++--  
NetworkPkg/IpSecDxe/Ikev2/Exchange.c |  10 ++-
 NetworkPkg/IpSecDxe/Ikev2/Payload.c  |   3 +
 NetworkPkg/IpSecDxe/Ikev2/Sa.c   | 163 ++-
 5 files changed, 188 insertions(+), 13 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/IkePacket.c b/NetworkPkg/IpSecDxe/IkePacket.c 
index 8fd395d..d5a938e 100644
--- a/NetworkPkg/IpSecDxe/IkePacket.c
+++ b/NetworkPkg/IpSecDxe/IkePacket.c
@@ -1,9 +1,9 @@
 /** @file
   IKE Packet related operation.
 
-  Copyright (c) 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights 
+ reserved.
 
   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.
@@ -201,11 +201,13 @@ IkeNetbufFromPacket (
 //
 // Convert Host order to Network order for IKE_PACKET header and payloads
 // Encryption payloads if needed
 //
 if (((IKEV2_SESSION_COMMON *) SessionCommon)->IkeVer == 2) {
-  Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) SessionCommon, IkePacket, 
IkeType);
+  if (EFI_ERROR (Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) 
SessionCommon, IkePacket, IkeType))) {
+return NULL;
+  }
 } else {
   //
   //If IKEv1 support, check it here.
   //
   return NULL;
diff --git a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c 
b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
index d3859e2..1f0199b 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
@@ -1,9 +1,9 @@
 /** @file
   The operations for Child SA.
 
-  Copyright (c) 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights 
+ reserved.
 
   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.
@@ -37,22 +37,25 @@ Ikev2CreateChildGenerator (
   IKEV2_CHILD_SA_SESSION  *ChildSaSession;
   IKEV2_SA_SESSION*IkeSaSession;
   IKE_PACKET  *IkePacket;
   IKE_PAYLOAD *NotifyPayload;
   UINT32  *MessageId;
+
+  NotifyPayload   = NULL;
+  MessageId   = NULL;
   
   ChildSaSession  = (IKEV2_CHILD_SA_SESSION *) SaSession;
-  IkePacket   = IkePacketAlloc();
-  MessageId   = NULL;
-
-  if (IkePacket == NULL) {
+  if (ChildSaSession == NULL) {
 return NULL;
   }
-  if (ChildSaSession == NULL) {
+
+  IkePacket   = IkePacketAlloc();
+  if (IkePacket == NULL) {
 return NULL;
   }
 
+
   if (Context != NULL) {
 MessageId = (UINT32 *) Context;
   }
   
   IkePacket->Header->Version  = (UINT8) (2 << 4);
@@ -111,10 +114,14 @@ Ikev2CreateChildGenerator (
 IKEV2_NOTIFICATION_NO_ADDITIONAL_SAS,
 NULL,
 NULL,
 0
 );
+  if (NotifyPayload == NULL) { 
+IkePacketFree (IkePacket);
+return NULL;
+  }
 
   IKE_PACKET_APPEND_PAYLOAD (IkePacket, NotifyPayload);
   //
   // TODO: Support the CREATE_CHILD_SA exchange. 
   //
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Exchange.c 
b/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
index 9d58ab0a..1eddbfb 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
@@ -1,9 +1,9 @@
 /** @file
   The general interfaces of the IKEv2.
 
-  Copyright (c) 2010 - 2015, Intel Corporation. All

[edk2] [Patch] NetworkPkg: Avoid potential NULL pointer dereference

2016-06-24 Thread Jiaxin Wu

The commit of 6b16c9e7 removes ASSERT and use error handling
in IpSecDxe driver, but may cause the potential NULL pointer
dereference. So, this patch is used to avoid NULL pointer
dereference.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/IpSecDxe/IkePacket.c  |   6 +-
 NetworkPkg/IpSecDxe/Ikev2/ChildSa.c  |  19 ++--
 NetworkPkg/IpSecDxe/Ikev2/Exchange.c |  10 ++-
 NetworkPkg/IpSecDxe/Ikev2/Payload.c  |   3 +
 NetworkPkg/IpSecDxe/Ikev2/Sa.c   | 163 ++-
 5 files changed, 188 insertions(+), 13 deletions(-)

diff --git a/NetworkPkg/IpSecDxe/IkePacket.c b/NetworkPkg/IpSecDxe/IkePacket.c
index 8fd395d..d5a938e 100644
--- a/NetworkPkg/IpSecDxe/IkePacket.c
+++ b/NetworkPkg/IpSecDxe/IkePacket.c
@@ -1,9 +1,9 @@
 /** @file
   IKE Packet related operation.
 
-  Copyright (c) 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
 
   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.
@@ -201,11 +201,13 @@ IkeNetbufFromPacket (
 //
 // Convert Host order to Network order for IKE_PACKET header and payloads
 // Encryption payloads if needed
 //
 if (((IKEV2_SESSION_COMMON *) SessionCommon)->IkeVer == 2) {
-  Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) SessionCommon, IkePacket, 
IkeType);
+  if (EFI_ERROR (Ikev2EncodePacket ((IKEV2_SESSION_COMMON *) 
SessionCommon, IkePacket, IkeType))) {
+return NULL;
+  }
 } else {
   //
   //If IKEv1 support, check it here.
   //
   return NULL;
diff --git a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c 
b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
index d3859e2..1f0199b 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/ChildSa.c
@@ -1,9 +1,9 @@
 /** @file
   The operations for Child SA.
 
-  Copyright (c) 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
 
   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.
@@ -37,22 +37,25 @@ Ikev2CreateChildGenerator (
   IKEV2_CHILD_SA_SESSION  *ChildSaSession;
   IKEV2_SA_SESSION*IkeSaSession;
   IKE_PACKET  *IkePacket;
   IKE_PAYLOAD *NotifyPayload;
   UINT32  *MessageId;
+
+  NotifyPayload   = NULL;
+  MessageId   = NULL;
   
   ChildSaSession  = (IKEV2_CHILD_SA_SESSION *) SaSession;
-  IkePacket   = IkePacketAlloc();
-  MessageId   = NULL;
-
-  if (IkePacket == NULL) {
+  if (ChildSaSession == NULL) {
 return NULL;
   }
-  if (ChildSaSession == NULL) {
+
+  IkePacket   = IkePacketAlloc();
+  if (IkePacket == NULL) {
 return NULL;
   }
 
+
   if (Context != NULL) {
 MessageId = (UINT32 *) Context;
   }
   
   IkePacket->Header->Version  = (UINT8) (2 << 4);
@@ -111,10 +114,14 @@ Ikev2CreateChildGenerator (
 IKEV2_NOTIFICATION_NO_ADDITIONAL_SAS,
 NULL,
 NULL,
 0
 );
+  if (NotifyPayload == NULL) { 
+IkePacketFree (IkePacket);
+return NULL;
+  }
 
   IKE_PACKET_APPEND_PAYLOAD (IkePacket, NotifyPayload);
   //
   // TODO: Support the CREATE_CHILD_SA exchange. 
   // 
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Exchange.c 
b/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
index 9d58ab0a..1eddbfb 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Exchange.c
@@ -1,9 +1,9 @@
 /** @file
   The general interfaces of the IKEv2.
 
-  Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
 
   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.
@@ -493,10 +493,14 @@ Ikev2HandleSa (
 //
 ASSERT (IsListEmpty (>ChildSaSessionList) &&
 IsListEmpty (>ChildSaEstablishSessionList));
 
 ChildSaSession = Ikev2ChildSaSessionCreate (IkeSaSession, UdpService);
+if (ChildSaSession == NULL) {
+  goto ON_ERROR;
+}
+
 ChildSaCommon  = >SessionCommon;
   }
 
   //
   // Parse the IKE request packet according to the auth method and current 
state.
@@ -517,10 +521,14 @@

Re: [edk2] [Patch] NetworkPkg: Avoid potential NULL pointer dereference

Re: [edk2] [Patch] NetworkPkg: Avoid potential NULL pointer dereference

[edk2] [Patch] NetworkPkg: Avoid potential NULL pointer dereference

3 matches

Site Navigation

Mail list logo

Footer information