From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web12.9725.1585917368051209461 for ; Fri, 03 Apr 2020 05:36:08 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: linux.intel.com, ip: 192.55.52.151, mailfrom: maciej.rabeda@linux.intel.com) IronPort-SDR: PBE6llMs/LIEyykBUNdr695Alb5aRVI+sWZ4OOblyxNdJ+rlWp1+JbhW53+3QCxA6zNlJr0C9W FJO1uiOnM49Q== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Apr 2020 05:36:07 -0700 IronPort-SDR: PNq4bEZiVL/RczD1b5IuZo8sPptM9uexTlVunz00zcrRPEk7OLzN0lRzHl4/2bNl2qXBWOYVSG LkMpxJ9gH6CA== X-IronPort-AV: E=Sophos;i="5.72,339,1580803200"; d="scan'208";a="423511702" Received: from mrabeda-mobl.ger.corp.intel.com (HELO [10.213.20.147]) ([10.213.20.147]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Apr 2020 05:36:06 -0700 Subject: Re: [edk2-devel] [PATCH v2] NetworkPkg/Ip6Dxe: Fix ASSERT logic in Ip6ProcessRouterAdvertise() To: devel@edk2.groups.io, jiaxin.wu@intel.com References: <20200402091434.1668-1-maciej.rabeda@linux.intel.com> <895558F6EA4E3B41AC93A00D163B727417061717@SHSMSX107.ccr.corp.intel.com> From: "Maciej Rabeda" Message-ID: <22b2f78d-45bb-7b8b-c03d-17e0686478cb@linux.intel.com> Date: Fri, 3 Apr 2020 14:36:04 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <895558F6EA4E3B41AC93A00D163B727417061717@SHSMSX107.ccr.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: pl Hey Jiaxin! Thanks for the review, I have already merged this patch based on Siyuan's review and Laszlo's test :) Also, big thank you for handling the HTTP static IP thread. - Maciej On 03-Apr-20 09:21, Wu, Jiaxin wrote: > Reviewed-by: Jiaxin Wu > > >> -----Original Message----- >> From: Maciej Rabeda >> Sent: Thursday, April 2, 2020 5:15 PM >> To: devel@edk2.groups.io >> Cc: Wu, Jiaxin ; Fu, Siyuan ; >> Laszlo Ersek >> Subject: [PATCH v2] NetworkPkg/Ip6Dxe: Fix ASSERT logic in >> Ip6ProcessRouterAdvertise() >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2655 >> >> This patch fixes reversed logic of recently added ASSERTs which should >> ensure that Ip6IsNDOptionValid() implementation properly reacts to invalid >> packets. >> >> Cc: Jiaxin Wu >> Cc: Siyuan Fu >> Signed-off-by: Maciej Rabeda >> Reviewed-by: Siyuan Fu >> Tested-by: Laszlo Ersek >> Fixes: 9c20342eed70ec99ec50cd73cb81804299f05403 >> --- >> NetworkPkg/Ip6Dxe/Ip6Nd.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/NetworkPkg/Ip6Dxe/Ip6Nd.c b/NetworkPkg/Ip6Dxe/Ip6Nd.c >> index fd7f60b2f92c..0780a98cb325 100644 >> --- a/NetworkPkg/Ip6Dxe/Ip6Nd.c >> +++ b/NetworkPkg/Ip6Dxe/Ip6Nd.c >> @@ -2111,7 +2111,7 @@ Ip6ProcessRouterAdvertise ( >> // Option size validity ensured by Ip6IsNDOptionValid(). >> >> // >> >> ASSERT (LinkLayerOption.Length != 0); >> >> - ASSERT (Offset + (UINT32) LinkLayerOption.Length * 8 >= (UINT32) Head- >>> PayloadLength); >> + ASSERT (Offset + (UINT32) LinkLayerOption.Length * 8 <= (UINT32) >> Head->PayloadLength); >> >> >> >> ZeroMem (&LinkLayerAddress, sizeof (EFI_MAC_ADDRESS)); >> >> CopyMem (&LinkLayerAddress, LinkLayerOption.EtherAddr, 6); >> >> @@ -2164,7 +2164,7 @@ Ip6ProcessRouterAdvertise ( >> // Option size validity ensured by Ip6IsNDOptionValid(). >> >> // >> >> ASSERT (PrefixOption.Length == 4); >> >> - ASSERT (Offset + (UINT32) PrefixOption.Length * 8 >= (UINT32) Head- >>> PayloadLength); >> + ASSERT (Offset + (UINT32) PrefixOption.Length * 8 <= (UINT32) Head- >>> PayloadLength); >> >> >> PrefixOption.ValidLifetime = NTOHL (PrefixOption.ValidLifetime); >> >> PrefixOption.PreferredLifetime = NTOHL >> (PrefixOption.PreferredLifetime); >> >> @@ -2334,7 +2334,7 @@ Ip6ProcessRouterAdvertise ( >> // Option size validity ensured by Ip6IsNDOptionValid(). >> >> // >> >> ASSERT (MTUOption.Length == 1); >> >> - ASSERT (Offset + (UINT32) MTUOption.Length * 8 >= (UINT32) Head- >>> PayloadLength); >> + ASSERT (Offset + (UINT32) MTUOption.Length * 8 <= (UINT32) Head- >>> PayloadLength); >> >> >> // >> >> // Use IPv6 minimum link MTU 1280 bytes as the maximum packet size in >> order >> >> -- >> 2.24.0.windows.2 > > >