public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/2] MdeModulePkg/Ip4Dxe: Issue fix for Ip4Dxe
@ 2016-11-11  5:18 Jiaxin Wu
  2016-11-11  5:18 ` [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check Jiaxin Wu
  2016-11-11  5:18 ` [PATCH v2 2/2] MdeModulePkg/Ip4Dxe: Correct the return status Jiaxin Wu
  0 siblings, 2 replies; 8+ messages in thread
From: Jiaxin Wu @ 2016-11-11  5:18 UTC (permalink / raw)
  To: edk2-devel; +Cc: Santhapur Naveen, Laszlo Ersek, Ye Ting, Fu Siyuan

Cc: Santhapur Naveen <naveens@amiindia.co.in>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

Jiaxin Wu (2):
  MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check
  MdeModulePkg/Ip4Dxe: Correct the return status

 .../Universal/Network/Ip4Dxe/Ip4Config2Impl.c        | 20 ++++++++++----------
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c        |  8 +++++---
 2 files changed, 15 insertions(+), 13 deletions(-)

-- 
1.9.5.msysgit.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check
  2016-11-11  5:18 [PATCH v2 0/2] MdeModulePkg/Ip4Dxe: Issue fix for Ip4Dxe Jiaxin Wu
@ 2016-11-11  5:18 ` Jiaxin Wu
  2016-11-11 12:42   ` Laszlo Ersek
  2016-11-14  6:51   ` Ye, Ting
  2016-11-11  5:18 ` [PATCH v2 2/2] MdeModulePkg/Ip4Dxe: Correct the return status Jiaxin Wu
  1 sibling, 2 replies; 8+ messages in thread
From: Jiaxin Wu @ 2016-11-11  5:18 UTC (permalink / raw)
  To: edk2-devel; +Cc: Santhapur Naveen, Laszlo Ersek, Ye Ting, Fu Siyuan

v2:
* Separate out the return status fix.
* Replace IP4_MASK_MAX with IP4_MASK_MAX.
* Remove the ON_EXIT label.

This patch is used to add the wrong/invalid subnet check.

Cc: Santhapur Naveen <naveens@amiindia.co.in>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 +++++++---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c          |  8 +++++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
index a931bb3..5b01b35 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
@@ -1253,10 +1253,17 @@ Ip4Config2SetMaunualAddress (
     return EFI_WRITE_PROTECTED;
   }
 
   NewAddress = *((EFI_IP4_CONFIG2_MANUAL_ADDRESS *) Data);
 
+  StationAddress = EFI_NTOHL (NewAddress.Address);
+  SubnetMask = EFI_NTOHL (NewAddress.SubnetMask);
+
+  if (NetGetMaskLength (SubnetMask) == IP4_MASK_NUM) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   //
   // Store the new data, and init the DataItem status to EFI_NOT_READY because
   // we may have an asynchronous configuration process.
   //
   Ptr = AllocateCopyPool (DataSize, Data);
@@ -1271,13 +1278,10 @@ Ip4Config2SetMaunualAddress (
   
   DataItem->Data.Ptr = Ptr;
   DataItem->DataSize = DataSize;
   DataItem->Status   = EFI_NOT_READY;
 
-  StationAddress = EFI_NTOHL (NewAddress.Address);
-  SubnetMask = EFI_NTOHL (NewAddress.SubnetMask);
-
   IpSb->Reconfig = TRUE;
   Status = Ip4Config2SetDefaultAddr (IpSb, StationAddress, SubnetMask);
   if (EFI_ERROR (Status)) {
     goto ON_EXIT;
   }  
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
index 9cd5dd5..b0cc6a3 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
@@ -562,10 +562,15 @@ Ip4SetAddress (
   EFI_STATUS                Status;
   INTN                      Len;
 
   NET_CHECK_SIGNATURE (Interface, IP4_INTERFACE_SIGNATURE);
 
+  Len = NetGetMaskLength (SubnetMask);
+  if (Len == IP4_MASK_NUM) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   //
   // Set the ip/netmask, then compute the subnet broadcast
   // and network broadcast for easy access. When computing
   // nework broadcast, the subnet mask is most like longer
   // than the default netmask (not subneted) as defined in
@@ -573,13 +578,10 @@ Ip4SetAddress (
   // networks, use the subnet's mask instead.
   //
   Interface->Ip             = IpAddr;
   Interface->SubnetMask     = SubnetMask;
   Interface->SubnetBrdcast  = (IpAddr | ~SubnetMask);
-
-  Len                       = NetGetMaskLength (SubnetMask);
-  ASSERT (Len <= IP4_MASK_MAX);
   Interface->NetBrdcast     = (IpAddr | ~SubnetMask);
 
   //
   // Do clean up for Arp child
   //
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] MdeModulePkg/Ip4Dxe: Correct the return status
  2016-11-11  5:18 [PATCH v2 0/2] MdeModulePkg/Ip4Dxe: Issue fix for Ip4Dxe Jiaxin Wu
  2016-11-11  5:18 ` [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check Jiaxin Wu
@ 2016-11-11  5:18 ` Jiaxin Wu
  2016-11-11 12:45   ` Laszlo Ersek
  1 sibling, 1 reply; 8+ messages in thread
From: Jiaxin Wu @ 2016-11-11  5:18 UTC (permalink / raw)
  To: edk2-devel; +Cc: Santhapur Naveen, Laszlo Ersek, Ye Ting, Fu Siyuan

This patch made the following change:
* DataItem->Status should be updated to the status code.
* Data should not be freed if EFI_NOT_READY returned.

Cc: Santhapur Naveen <naveens@amiindia.co.in>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
index 5b01b35..88ead9d 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
@@ -1280,25 +1280,21 @@ Ip4Config2SetMaunualAddress (
   DataItem->DataSize = DataSize;
   DataItem->Status   = EFI_NOT_READY;
 
   IpSb->Reconfig = TRUE;
   Status = Ip4Config2SetDefaultAddr (IpSb, StationAddress, SubnetMask);
-  if (EFI_ERROR (Status)) {
-    goto ON_EXIT;
-  }  
 
-  DataItem->Status = EFI_SUCCESS;   
+  DataItem->Status = Status; 
 
-ON_EXIT:
-  if (EFI_ERROR (DataItem->Status)) {
+  if (EFI_ERROR (DataItem->Status) && DataItem->Status != EFI_NOT_READY) {
     if (Ptr != NULL) {
       FreePool (Ptr);
     }
     DataItem->Data.Ptr = NULL; 
   }
 
-  return EFI_SUCCESS;
+  return Status;
 }
 
 /**
   The work function is to set the gateway addresses manually for the EFI IPv4 
   network stack that is running on the communication device that this EFI IPv4 
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check
  2016-11-11  5:18 ` [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check Jiaxin Wu
@ 2016-11-11 12:42   ` Laszlo Ersek
  2016-11-14  6:51   ` Ye, Ting
  1 sibling, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2016-11-11 12:42 UTC (permalink / raw)
  To: Jiaxin Wu, edk2-devel; +Cc: Ye Ting, Fu Siyuan, Santhapur Naveen

On 11/11/16 06:18, Jiaxin Wu wrote:
> v2:
> * Separate out the return status fix.
> * Replace IP4_MASK_MAX with IP4_MASK_MAX.
> * Remove the ON_EXIT label.
> 
> This patch is used to add the wrong/invalid subnet check.
> 
> Cc: Santhapur Naveen <naveens@amiindia.co.in>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 +++++++---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c          |  8 +++++---
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> index a931bb3..5b01b35 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> @@ -1253,10 +1253,17 @@ Ip4Config2SetMaunualAddress (
>      return EFI_WRITE_PROTECTED;
>    }
>  
>    NewAddress = *((EFI_IP4_CONFIG2_MANUAL_ADDRESS *) Data);
>  
> +  StationAddress = EFI_NTOHL (NewAddress.Address);
> +  SubnetMask = EFI_NTOHL (NewAddress.SubnetMask);
> +
> +  if (NetGetMaskLength (SubnetMask) == IP4_MASK_NUM) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    //
>    // Store the new data, and init the DataItem status to EFI_NOT_READY because
>    // we may have an asynchronous configuration process.
>    //
>    Ptr = AllocateCopyPool (DataSize, Data);
> @@ -1271,13 +1278,10 @@ Ip4Config2SetMaunualAddress (
>    
>    DataItem->Data.Ptr = Ptr;
>    DataItem->DataSize = DataSize;
>    DataItem->Status   = EFI_NOT_READY;
>  
> -  StationAddress = EFI_NTOHL (NewAddress.Address);
> -  SubnetMask = EFI_NTOHL (NewAddress.SubnetMask);
> -
>    IpSb->Reconfig = TRUE;
>    Status = Ip4Config2SetDefaultAddr (IpSb, StationAddress, SubnetMask);
>    if (EFI_ERROR (Status)) {
>      goto ON_EXIT;
>    }  
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> index 9cd5dd5..b0cc6a3 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> @@ -562,10 +562,15 @@ Ip4SetAddress (
>    EFI_STATUS                Status;
>    INTN                      Len;
>  
>    NET_CHECK_SIGNATURE (Interface, IP4_INTERFACE_SIGNATURE);
>  
> +  Len = NetGetMaskLength (SubnetMask);
> +  if (Len == IP4_MASK_NUM) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    //
>    // Set the ip/netmask, then compute the subnet broadcast
>    // and network broadcast for easy access. When computing
>    // nework broadcast, the subnet mask is most like longer
>    // than the default netmask (not subneted) as defined in
> @@ -573,13 +578,10 @@ Ip4SetAddress (
>    // networks, use the subnet's mask instead.
>    //
>    Interface->Ip             = IpAddr;
>    Interface->SubnetMask     = SubnetMask;
>    Interface->SubnetBrdcast  = (IpAddr | ~SubnetMask);
> -
> -  Len                       = NetGetMaskLength (SubnetMask);
> -  ASSERT (Len <= IP4_MASK_MAX);
>    Interface->NetBrdcast     = (IpAddr | ~SubnetMask);
>  
>    //
>    // Do clean up for Arp child
>    //
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/2] MdeModulePkg/Ip4Dxe: Correct the return status
  2016-11-11  5:18 ` [PATCH v2 2/2] MdeModulePkg/Ip4Dxe: Correct the return status Jiaxin Wu
@ 2016-11-11 12:45   ` Laszlo Ersek
  0 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2016-11-11 12:45 UTC (permalink / raw)
  To: Jiaxin Wu, edk2-devel; +Cc: Ye Ting, Fu Siyuan, Santhapur Naveen

On 11/11/16 06:18, Jiaxin Wu wrote:
> This patch made the following change:
> * DataItem->Status should be updated to the status code.
> * Data should not be freed if EFI_NOT_READY returned.
> 
> Cc: Santhapur Naveen <naveens@amiindia.co.in>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> index 5b01b35..88ead9d 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> @@ -1280,25 +1280,21 @@ Ip4Config2SetMaunualAddress (
>    DataItem->DataSize = DataSize;
>    DataItem->Status   = EFI_NOT_READY;
>  
>    IpSb->Reconfig = TRUE;
>    Status = Ip4Config2SetDefaultAddr (IpSb, StationAddress, SubnetMask);
> -  if (EFI_ERROR (Status)) {
> -    goto ON_EXIT;
> -  }  
>  
> -  DataItem->Status = EFI_SUCCESS;   
> +  DataItem->Status = Status; 
>  
> -ON_EXIT:
> -  if (EFI_ERROR (DataItem->Status)) {
> +  if (EFI_ERROR (DataItem->Status) && DataItem->Status != EFI_NOT_READY) {
>      if (Ptr != NULL) {
>        FreePool (Ptr);
>      }
>      DataItem->Data.Ptr = NULL; 
>    }
>  
> -  return EFI_SUCCESS;
> +  return Status;
>  }
>  
>  /**
>    The work function is to set the gateway addresses manually for the EFI IPv4 
>    network stack that is running on the communication device that this EFI IPv4 
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check
  2016-11-11  5:18 ` [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check Jiaxin Wu
  2016-11-11 12:42   ` Laszlo Ersek
@ 2016-11-14  6:51   ` Ye, Ting
  2016-11-14  7:00     ` Wu, Jiaxin
  1 sibling, 1 reply; 8+ messages in thread
From: Ye, Ting @ 2016-11-14  6:51 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org
  Cc: Santhapur Naveen, Laszlo Ersek, Fu, Siyuan

Hi Jiaxin,

It looks the check-in log is incorrect. 
 -- Any typo in item 2?
-- Is item 3 part of  patch 2/2? 

Please double check, thanks.

Best Regards,
Ting

-----Original Message-----
From: Wu, Jiaxin 
Sent: Friday, November 11, 2016 1:19 PM
To: edk2-devel@lists.01.org
Cc: Santhapur Naveen <naveens@amiindia.co.in>; Laszlo Ersek <lersek@redhat.com>; Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
Subject: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check

v2:
* Separate out the return status fix.
* Replace IP4_MASK_MAX with IP4_MASK_MAX.
* Remove the ON_EXIT label.

This patch is used to add the wrong/invalid subnet check.

Cc: Santhapur Naveen <naveens@amiindia.co.in>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 +++++++---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c          |  8 +++++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
index a931bb3..5b01b35 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
@@ -1253,10 +1253,17 @@ Ip4Config2SetMaunualAddress (
     return EFI_WRITE_PROTECTED;
   }
 
   NewAddress = *((EFI_IP4_CONFIG2_MANUAL_ADDRESS *) Data);
 
+  StationAddress = EFI_NTOHL (NewAddress.Address);  SubnetMask = 
+ EFI_NTOHL (NewAddress.SubnetMask);
+
+  if (NetGetMaskLength (SubnetMask) == IP4_MASK_NUM) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   //
   // Store the new data, and init the DataItem status to EFI_NOT_READY because
   // we may have an asynchronous configuration process.
   //
   Ptr = AllocateCopyPool (DataSize, Data); @@ -1271,13 +1278,10 @@ Ip4Config2SetMaunualAddress (
   
   DataItem->Data.Ptr = Ptr;
   DataItem->DataSize = DataSize;
   DataItem->Status   = EFI_NOT_READY;
 
-  StationAddress = EFI_NTOHL (NewAddress.Address);
-  SubnetMask = EFI_NTOHL (NewAddress.SubnetMask);
-
   IpSb->Reconfig = TRUE;
   Status = Ip4Config2SetDefaultAddr (IpSb, StationAddress, SubnetMask);
   if (EFI_ERROR (Status)) {
     goto ON_EXIT;
   }
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
index 9cd5dd5..b0cc6a3 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
@@ -562,10 +562,15 @@ Ip4SetAddress (
   EFI_STATUS                Status;
   INTN                      Len;
 
   NET_CHECK_SIGNATURE (Interface, IP4_INTERFACE_SIGNATURE);
 
+  Len = NetGetMaskLength (SubnetMask);
+  if (Len == IP4_MASK_NUM) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   //
   // Set the ip/netmask, then compute the subnet broadcast
   // and network broadcast for easy access. When computing
   // nework broadcast, the subnet mask is most like longer
   // than the default netmask (not subneted) as defined in @@ -573,13 +578,10 @@ Ip4SetAddress (
   // networks, use the subnet's mask instead.
   //
   Interface->Ip             = IpAddr;
   Interface->SubnetMask     = SubnetMask;
   Interface->SubnetBrdcast  = (IpAddr | ~SubnetMask);
-
-  Len                       = NetGetMaskLength (SubnetMask);
-  ASSERT (Len <= IP4_MASK_MAX);
   Interface->NetBrdcast     = (IpAddr | ~SubnetMask);
 
   //
   // Do clean up for Arp child
   //
--
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check
  2016-11-14  6:51   ` Ye, Ting
@ 2016-11-14  7:00     ` Wu, Jiaxin
  2016-11-14  7:16       ` Ye, Ting
  0 siblings, 1 reply; 8+ messages in thread
From: Wu, Jiaxin @ 2016-11-14  7:00 UTC (permalink / raw)
  To: Ye, Ting, edk2-devel@lists.01.org
  Cc: Santhapur Naveen, Laszlo Ersek, Fu, Siyuan

Hi Ting,

Thanks for catch.  Item 2 should be: * Replace IP4_MASK_MAX with IP4_MASK_NUM.
Item 3 is part of patch 2/2.

I will correct it when I commit the patch.

Thanks
Jiaxin.

> -----Original Message-----
> From: Ye, Ting
> Sent: Monday, November 14, 2016 2:52 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Santhapur Naveen <naveens@amiindia.co.in>; Laszlo Ersek
> <lersek@redhat.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: RE: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet
> check
> 
> Hi Jiaxin,
> 
> It looks the check-in log is incorrect.
>  -- Any typo in item 2?
> -- Is item 3 part of  patch 2/2?
> 
> Please double check, thanks.
> 
> Best Regards,
> Ting
> 
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Friday, November 11, 2016 1:19 PM
> To: edk2-devel@lists.01.org
> Cc: Santhapur Naveen <naveens@amiindia.co.in>; Laszlo Ersek
> <lersek@redhat.com>; Ye, Ting <ting.ye@intel.com>; Fu, Siyuan
> <siyuan.fu@intel.com>
> Subject: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet
> check
> 
> v2:
> * Separate out the return status fix.
> * Replace IP4_MASK_MAX with IP4_MASK_MAX.
> * Remove the ON_EXIT label.
> 
> This patch is used to add the wrong/invalid subnet check.
> 
> Cc: Santhapur Naveen <naveens@amiindia.co.in>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 +++++++---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c          |  8 +++++---
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> index a931bb3..5b01b35 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> @@ -1253,10 +1253,17 @@ Ip4Config2SetMaunualAddress (
>      return EFI_WRITE_PROTECTED;
>    }
> 
>    NewAddress = *((EFI_IP4_CONFIG2_MANUAL_ADDRESS *) Data);
> 
> +  StationAddress = EFI_NTOHL (NewAddress.Address);  SubnetMask =
> + EFI_NTOHL (NewAddress.SubnetMask);
> +
> +  if (NetGetMaskLength (SubnetMask) == IP4_MASK_NUM) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    //
>    // Store the new data, and init the DataItem status to EFI_NOT_READY
> because
>    // we may have an asynchronous configuration process.
>    //
>    Ptr = AllocateCopyPool (DataSize, Data); @@ -1271,13 +1278,10 @@
> Ip4Config2SetMaunualAddress (
> 
>    DataItem->Data.Ptr = Ptr;
>    DataItem->DataSize = DataSize;
>    DataItem->Status   = EFI_NOT_READY;
> 
> -  StationAddress = EFI_NTOHL (NewAddress.Address);
> -  SubnetMask = EFI_NTOHL (NewAddress.SubnetMask);
> -
>    IpSb->Reconfig = TRUE;
>    Status = Ip4Config2SetDefaultAddr (IpSb, StationAddress, SubnetMask);
>    if (EFI_ERROR (Status)) {
>      goto ON_EXIT;
>    }
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> index 9cd5dd5..b0cc6a3 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> @@ -562,10 +562,15 @@ Ip4SetAddress (
>    EFI_STATUS                Status;
>    INTN                      Len;
> 
>    NET_CHECK_SIGNATURE (Interface, IP4_INTERFACE_SIGNATURE);
> 
> +  Len = NetGetMaskLength (SubnetMask);
> +  if (Len == IP4_MASK_NUM) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    //
>    // Set the ip/netmask, then compute the subnet broadcast
>    // and network broadcast for easy access. When computing
>    // nework broadcast, the subnet mask is most like longer
>    // than the default netmask (not subneted) as defined in @@ -573,13 +578,10
> @@ Ip4SetAddress (
>    // networks, use the subnet's mask instead.
>    //
>    Interface->Ip             = IpAddr;
>    Interface->SubnetMask     = SubnetMask;
>    Interface->SubnetBrdcast  = (IpAddr | ~SubnetMask);
> -
> -  Len                       = NetGetMaskLength (SubnetMask);
> -  ASSERT (Len <= IP4_MASK_MAX);
>    Interface->NetBrdcast     = (IpAddr | ~SubnetMask);
> 
>    //
>    // Do clean up for Arp child
>    //
> --
> 1.9.5.msysgit.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check
  2016-11-14  7:00     ` Wu, Jiaxin
@ 2016-11-14  7:16       ` Ye, Ting
  0 siblings, 0 replies; 8+ messages in thread
From: Ye, Ting @ 2016-11-14  7:16 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org
  Cc: Santhapur Naveen, Laszlo Ersek, Fu, Siyuan

Sounds good. 
Reviewed-by: Ye Ting <ting.ye@intel.com> 

Thanks,
Ting

-----Original Message-----
From: Wu, Jiaxin 
Sent: Monday, November 14, 2016 3:01 PM
To: Ye, Ting <ting.ye@intel.com>; edk2-devel@lists.01.org
Cc: Santhapur Naveen <naveens@amiindia.co.in>; Laszlo Ersek <lersek@redhat.com>; Fu, Siyuan <siyuan.fu@intel.com>
Subject: RE: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check

Hi Ting,

Thanks for catch.  Item 2 should be: * Replace IP4_MASK_MAX with IP4_MASK_NUM.
Item 3 is part of patch 2/2.

I will correct it when I commit the patch.

Thanks
Jiaxin.

> -----Original Message-----
> From: Ye, Ting
> Sent: Monday, November 14, 2016 2:52 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Santhapur Naveen <naveens@amiindia.co.in>; Laszlo Ersek 
> <lersek@redhat.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: RE: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid 
> subnet check
> 
> Hi Jiaxin,
> 
> It looks the check-in log is incorrect.
>  -- Any typo in item 2?
> -- Is item 3 part of  patch 2/2?
> 
> Please double check, thanks.
> 
> Best Regards,
> Ting
> 
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Friday, November 11, 2016 1:19 PM
> To: edk2-devel@lists.01.org
> Cc: Santhapur Naveen <naveens@amiindia.co.in>; Laszlo Ersek 
> <lersek@redhat.com>; Ye, Ting <ting.ye@intel.com>; Fu, Siyuan 
> <siyuan.fu@intel.com>
> Subject: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet 
> check
> 
> v2:
> * Separate out the return status fix.
> * Replace IP4_MASK_MAX with IP4_MASK_MAX.
> * Remove the ON_EXIT label.
> 
> This patch is used to add the wrong/invalid subnet check.
> 
> Cc: Santhapur Naveen <naveens@amiindia.co.in>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 +++++++---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c          |  8 +++++---
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> index a931bb3..5b01b35 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> @@ -1253,10 +1253,17 @@ Ip4Config2SetMaunualAddress (
>      return EFI_WRITE_PROTECTED;
>    }
> 
>    NewAddress = *((EFI_IP4_CONFIG2_MANUAL_ADDRESS *) Data);
> 
> +  StationAddress = EFI_NTOHL (NewAddress.Address);  SubnetMask = 
> + EFI_NTOHL (NewAddress.SubnetMask);
> +
> +  if (NetGetMaskLength (SubnetMask) == IP4_MASK_NUM) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    //
>    // Store the new data, and init the DataItem status to 
> EFI_NOT_READY because
>    // we may have an asynchronous configuration process.
>    //
>    Ptr = AllocateCopyPool (DataSize, Data); @@ -1271,13 +1278,10 @@ 
> Ip4Config2SetMaunualAddress (
> 
>    DataItem->Data.Ptr = Ptr;
>    DataItem->DataSize = DataSize;
>    DataItem->Status   = EFI_NOT_READY;
> 
> -  StationAddress = EFI_NTOHL (NewAddress.Address);
> -  SubnetMask = EFI_NTOHL (NewAddress.SubnetMask);
> -
>    IpSb->Reconfig = TRUE;
>    Status = Ip4Config2SetDefaultAddr (IpSb, StationAddress, SubnetMask);
>    if (EFI_ERROR (Status)) {
>      goto ON_EXIT;
>    }
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> index 9cd5dd5..b0cc6a3 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> @@ -562,10 +562,15 @@ Ip4SetAddress (
>    EFI_STATUS                Status;
>    INTN                      Len;
> 
>    NET_CHECK_SIGNATURE (Interface, IP4_INTERFACE_SIGNATURE);
> 
> +  Len = NetGetMaskLength (SubnetMask);  if (Len == IP4_MASK_NUM) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    //
>    // Set the ip/netmask, then compute the subnet broadcast
>    // and network broadcast for easy access. When computing
>    // nework broadcast, the subnet mask is most like longer
>    // than the default netmask (not subneted) as defined in @@ -573,13 
> +578,10 @@ Ip4SetAddress (
>    // networks, use the subnet's mask instead.
>    //
>    Interface->Ip             = IpAddr;
>    Interface->SubnetMask     = SubnetMask;
>    Interface->SubnetBrdcast  = (IpAddr | ~SubnetMask);
> -
> -  Len                       = NetGetMaskLength (SubnetMask);
> -  ASSERT (Len <= IP4_MASK_MAX);
>    Interface->NetBrdcast     = (IpAddr | ~SubnetMask);
> 
>    //
>    // Do clean up for Arp child
>    //
> --
> 1.9.5.msysgit.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-11-14  7:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-11  5:18 [PATCH v2 0/2] MdeModulePkg/Ip4Dxe: Issue fix for Ip4Dxe Jiaxin Wu
2016-11-11  5:18 ` [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check Jiaxin Wu
2016-11-11 12:42   ` Laszlo Ersek
2016-11-14  6:51   ` Ye, Ting
2016-11-14  7:00     ` Wu, Jiaxin
2016-11-14  7:16       ` Ye, Ting
2016-11-11  5:18 ` [PATCH v2 2/2] MdeModulePkg/Ip4Dxe: Correct the return status Jiaxin Wu
2016-11-11 12:45   ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox