Re: [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check

["Ye, Ting" <ting.ye@intel.com>]

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