public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] NetworkPkg/Ip6Dxe: Fix ASSERT logic in Ip6ProcessRouterAdvertise()
@ 2020-04-02  9:14 Maciej Rabeda
  2020-04-03  7:21 ` Wu, Jiaxin
  0 siblings, 1 reply; 3+ messages in thread
From: Maciej Rabeda @ 2020-04-02  9:14 UTC (permalink / raw)
  To: devel; +Cc: Jiaxin Wu, Siyuan Fu, Laszlo Ersek

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 <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
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


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

* Re: [PATCH v2] NetworkPkg/Ip6Dxe: Fix ASSERT logic in Ip6ProcessRouterAdvertise()
  2020-04-02  9:14 [PATCH v2] NetworkPkg/Ip6Dxe: Fix ASSERT logic in Ip6ProcessRouterAdvertise() Maciej Rabeda
@ 2020-04-03  7:21 ` Wu, Jiaxin
  2020-04-03 12:36   ` [edk2-devel] " Maciej Rabeda
  0 siblings, 1 reply; 3+ messages in thread
From: Wu, Jiaxin @ 2020-04-03  7:21 UTC (permalink / raw)
  To: Maciej Rabeda, devel@edk2.groups.io; +Cc: Fu, Siyuan, Laszlo Ersek

Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>


> -----Original Message-----
> From: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Sent: Thursday, April 2, 2020 5:15 PM
> To: devel@edk2.groups.io
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> Laszlo Ersek <lersek@redhat.com>
> 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 <jiaxin.wu@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Signed-off-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
> Tested-by: Laszlo Ersek <lersek@redhat.com>
> 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


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

* Re: [edk2-devel] [PATCH v2] NetworkPkg/Ip6Dxe: Fix ASSERT logic in Ip6ProcessRouterAdvertise()
  2020-04-03  7:21 ` Wu, Jiaxin
@ 2020-04-03 12:36   ` Maciej Rabeda
  0 siblings, 0 replies; 3+ messages in thread
From: Maciej Rabeda @ 2020-04-03 12:36 UTC (permalink / raw)
  To: devel, jiaxin.wu

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 <jiaxin.wu@intel.com>
>
>
>> -----Original Message-----
>> From: Maciej Rabeda <maciej.rabeda@linux.intel.com>
>> Sent: Thursday, April 2, 2020 5:15 PM
>> To: devel@edk2.groups.io
>> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
>> Laszlo Ersek <lersek@redhat.com>
>> 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 <jiaxin.wu@intel.com>
>> Cc: Siyuan Fu <siyuan.fu@intel.com>
>> Signed-off-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
>> Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
>> Tested-by: Laszlo Ersek <lersek@redhat.com>
>> 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
>
> 
>


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

end of thread, other threads:[~2020-04-03 12:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-02  9:14 [PATCH v2] NetworkPkg/Ip6Dxe: Fix ASSERT logic in Ip6ProcessRouterAdvertise() Maciej Rabeda
2020-04-03  7:21 ` Wu, Jiaxin
2020-04-03 12:36   ` [edk2-devel] " Maciej Rabeda

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