From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (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 5DF0F802B0 for ; Wed, 15 Mar 2017 22:24:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1489641884; x=1521177884; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=eXeMfOKY6LSTx4J/5t4np8gwM2JdU6VDyvD1NFKZcco=; b=NdUIPIe3kMhPMrVi6/xpIrFi0Rb+o91yHkNnf8Z3GfOdVOag79OUisTt 1Q9K9XoSCO/VPlPzbTBadPS5i+ctgw==; Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Mar 2017 22:24:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,170,1486454400"; d="scan'208";a="75975507" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga005.jf.intel.com with ESMTP; 15 Mar 2017 22:24:43 -0700 Received: from fmsmsx126.amr.corp.intel.com (10.18.125.43) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 15 Mar 2017 22:24:43 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX126.amr.corp.intel.com (10.18.125.43) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 15 Mar 2017 22:24:43 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.20]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.204]) with mapi id 14.03.0248.002; Thu, 16 Mar 2017 13:24:39 +0800 From: "Wu, Jiaxin" To: "Hegde, Nagaraj P" , "edk2-devel@lists.01.org" CC: "Ye, Ting" , "Fu, Siyuan" Thread-Topic: [Patch] MdeModulePkg/Ip4Dxe: Add Ip/Netmask pair check for Ip4Config2 Thread-Index: AQHSnfZjybdmZFGNP0exHW6XJWt0RaGWRGUAgAAYL4CAAI190A== Date: Thu, 16 Mar 2017 05:24:39 +0000 Message-ID: <895558F6EA4E3B41AC93A00D163B7274162A39A4@SHSMSX103.ccr.corp.intel.com> References: <1489628458-16580-1-git-send-email-jiaxin.wu@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZWMyZmQ1MzMtNTc3NC00YTA2LWIzYTMtM2QzODVkOWQ3NDljIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IkVwV05xdjViQm5aWVVVYjgrXC82S0lHQm1PNkJFMTRoTkxsU1psT3pYZDZVPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch] MdeModulePkg/Ip4Dxe: Add Ip/Netmask pair check for Ip4Config2 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 05:24:44 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Nagaraj, You are right, but I'd like to update the patch to check it in Ip4Config2Se= tDefaultIf instead of Ip4SetAddress to avoid the previous interface free op= eration. After that, the check in Ip4SetAddress is needless.=20 Thanks, Jiaxin > -----Original Message----- > From: Hegde, Nagaraj P [mailto:nagaraj-p.hegde@hpe.com] > Sent: Thursday, March 16, 2017 12:40 PM > To: Wu, Jiaxin ; edk2-devel@lists.01.org > Cc: Ye, Ting ; Fu, Siyuan > Subject: RE: [Patch] MdeModulePkg/Ip4Dxe: Add Ip/Netmask pair check for > Ip4Config2 >=20 > We need to handle the case of DHCP server serving with a subnet of 0.0.0= .0. > When we set the Policy to Dhcp, we enter into Ip4Config2OnPolicyChanged, > which calls Ip4StartAutoConfig. Here we configure DHCP and we queue up > Ip4Config2OnDhcp4Complete to be called once DHCP is complete. In > Ip4Config2OnDhcp4Complete, we call Ip4Config2SetDefaultIf, which calls > Ip4Config2SetDefaultAddr, where we call Ip4SetAddress. Ip4SetAddress has > a similar check as Ip4Config2SetMaunualAddress: >=20 > Len =3D NetGetMaskLength (SubnetMask); > if (Len =3D=3D IP4_MASK_NUM) { > return EFI_INVALID_PARAMETER; > } >=20 > Shouldn't this also be updated? >=20 > Regards, > Nagaraj. >=20 > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Hegde, Nagaraj P > Sent: Thursday, March 16, 2017 8:43 AM > To: Jiaxin Wu ; edk2-devel@lists.01.org > Cc: Ye Ting ; Fu Siyuan > Subject: Re: [edk2] [Patch] MdeModulePkg/Ip4Dxe: Add Ip/Netmask pair > check for Ip4Config2 >=20 > Reviewed-by: Hegde, Nagaraj P >=20 > -----Original Message----- > From: Jiaxin Wu [mailto:jiaxin.wu@intel.com] > Sent: Thursday, March 16, 2017 7:11 AM > To: edk2-devel@lists.01.org > Cc: Hegde, Nagaraj P ; Subramanian, Sriram > ; Ye Ting ; Fu Siyuan > ; Wu Jiaxin > Subject: [Patch] MdeModulePkg/Ip4Dxe: Add Ip/Netmask pair check for > Ip4Config2 >=20 > Ip4config2 manual address setting doesn't check the validity of Ip/Netmas= k > pair, which leads to the invalid combination of Ip and Netmask setting. T= his > patch is to resolve this issue. >=20 > Cc: Hegde Nagaraj P > Cc: Subramanian Sriram > Cc: Ye Ting > Cc: Fu Siyuan > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Wu Jiaxin > --- > MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.c | 62 > +++++++++++++++++++++- > MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h | 21 +++++++- > .../Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 5 +- > MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c | 62 +---------------= -- > ---- > 4 files changed, 86 insertions(+), 64 deletions(-) >=20 > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.c > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.c > index 004a8bc..7c7d182 100644 > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.c > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.c > @@ -1,8 +1,8 @@ > /** @file >=20 > -Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.
> +Copyright (c) 2005 - 2017, 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 >=20 > @@ -265,5 +265,65 @@ Ip4NtohHead ( > Head->Src =3D NTOHL (Head->Src); > Head->Dst =3D NTOHL (Head->Dst); >=20 > return Head; > } > + > + > +/** > + Validate that Ip/Netmask pair is OK to be used as station > + address. Only continuous netmasks are supported. and check > + that StationAddress is a unicast address on the newtwork. > + > + @param[in] Ip The IP address to validate. > + @param[in] Netmask The netmaks of the IP. > + > + @retval TRUE The Ip/Netmask pair is valid. > + @retval FALSE The Ip/Netmask pair is invalid. > + > +**/ > +BOOLEAN > +Ip4StationAddressValid ( > + IN IP4_ADDR Ip, > + IN IP4_ADDR Netmask > + ) > +{ > + IP4_ADDR NetBrdcastMask; > + INTN Len; > + INTN Type; > + > + // > + // Only support the station address with 0.0.0.0/0 to enable DHCP clie= nt. > + // > + if (Netmask =3D=3D IP4_ALLZERO_ADDRESS) { > + return (BOOLEAN) (Ip =3D=3D IP4_ALLZERO_ADDRESS); } > + > + // > + // Only support the continuous net masks // if ((Len =3D > + NetGetMaskLength (Netmask)) =3D=3D (IP4_MASK_MAX + 1)) { > + return FALSE; > + } > + > + // > + // Station address can't be class D or class E address // if ((Type > + =3D NetGetIpClass (Ip)) > IP4_ADDR_CLASSC) { > + return FALSE; > + } > + > + // > + // Station address can't be subnet broadcast/net broadcast address // > + if ((Ip =3D=3D (Ip & Netmask)) || (Ip =3D=3D (Ip | ~Netmask))) { > + return FALSE; > + } > + > + NetBrdcastMask =3D gIp4AllMasks[MIN (Len, Type << 3)]; > + > + if (Ip =3D=3D (Ip | ~NetBrdcastMask)) { > + return FALSE; > + } > + > + return TRUE; > +} > \ No newline at end of file > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h > index d38857c..9689f37 100644 > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h > @@ -1,9 +1,9 @@ > /** @file > Common definition for IP4. >=20 > -Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.
> +Copyright (c) 2005 - 2017, 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 >=20 > @@ -199,6 +199,25 @@ Ip4GetMulticastMac ( IP4_HEAD * Ip4NtohHead ( > IN IP4_HEAD *Head > ); >=20 > + > +/** > + Validate that Ip/Netmask pair is OK to be used as station > + address. Only continuous netmasks are supported. and check > + that StationAddress is a unicast address on the newtwork. > + > + @param[in] Ip The IP address to validate. > + @param[in] Netmask The netmaks of the IP. > + > + @retval TRUE The Ip/Netmask pair is valid. > + @retval FALSE The Ip/Netmask pair is invalid. > + > +**/ > +BOOLEAN > +Ip4StationAddressValid ( > + IN IP4_ADDR Ip, > + IN IP4_ADDR Netmask > + ); > + > #endif > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c > index 6c7ac68..a5191d1 100644 > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c > @@ -1250,11 +1250,14 @@ Ip4Config2SetMaunualAddress ( > NewAddress =3D *((EFI_IP4_CONFIG2_MANUAL_ADDRESS *) Data); >=20 > StationAddress =3D EFI_NTOHL (NewAddress.Address); > SubnetMask =3D EFI_NTOHL (NewAddress.SubnetMask); >=20 > - if (NetGetMaskLength (SubnetMask) =3D=3D IP4_MASK_NUM) { > + // > + // Check whether the StationAddress/SubnetMask pair is valid. > + // > + if (!Ip4StationAddressValid (StationAddress, SubnetMask)) { > return EFI_INVALID_PARAMETER; > } >=20 > // > // Store the new data, and init the DataItem status to EFI_NOT_READY > because diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c > index 91f1a67..5aa3ea1 100644 > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c > @@ -1,8 +1,8 @@ > /** @file >=20 > -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2005 - 2017, 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 >=20 > @@ -808,70 +808,10 @@ Ip4CleanProtocol ( > return EFI_SUCCESS; > } >=20 >=20 > /** > - Validate that Ip/Netmask pair is OK to be used as station > - address. Only continuous netmasks are supported. and check > - that StationAddress is a unicast address on the newtwork. > - > - @param[in] Ip The IP address to validate. > - @param[in] Netmask The netmaks of the IP. > - > - @retval TRUE The Ip/Netmask pair is valid. > - @retval FALSE The Ip/Netmask pair is invalid. > - > -**/ > -BOOLEAN > -Ip4StationAddressValid ( > - IN IP4_ADDR Ip, > - IN IP4_ADDR Netmask > - ) > -{ > - IP4_ADDR NetBrdcastMask; > - INTN Len; > - INTN Type; > - > - // > - // Only support the station address with 0.0.0.0/0 to enable DHCP clie= nt. > - // > - if (Netmask =3D=3D IP4_ALLZERO_ADDRESS) { > - return (BOOLEAN) (Ip =3D=3D IP4_ALLZERO_ADDRESS); > - } > - > - // > - // Only support the continuous net masks > - // > - if ((Len =3D NetGetMaskLength (Netmask)) =3D=3D (IP4_MASK_MAX + 1)) { > - return FALSE; > - } > - > - // > - // Station address can't be class D or class E address > - // > - if ((Type =3D NetGetIpClass (Ip)) > IP4_ADDR_CLASSC) { > - return FALSE; > - } > - > - // > - // Station address can't be subnet broadcast/net broadcast address > - // > - if ((Ip =3D=3D (Ip & Netmask)) || (Ip =3D=3D (Ip | ~Netmask))) { > - return FALSE; > - } > - > - NetBrdcastMask =3D gIp4AllMasks[MIN (Len, Type << 3)]; > - > - if (Ip =3D=3D (Ip | ~NetBrdcastMask)) { > - return FALSE; > - } > - > - return TRUE; > -} > - > - > -/** > Assigns an IPv4 address and subnet mask to this EFI IPv4 Protocol driv= er > instance. >=20 > The Configure() function is used to set, change, or reset the operatio= nal > parameters and filter settings for this EFI IPv4 Protocol instance. Un= til these > parameters have been set, no network traffic can be sent or received b= y > this > -- > 1.9.5.msysgit.1 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel