[edk2-devel] [PATCH] NetworkPkg/Ip6Dxe: Fix ASSERT logic in Ip6ProcessRouterAdvertise()
Laszlo Ersek
lersek at redhat.com
Wed Apr 1 12:02:28 UTC 2020
Hi Maciej,
On 04/01/20 11:53, Maciej Rabeda wrote:
> 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 at intel.com>
> Cc: Siyuan Fu <siyuan.fu at intel.com>
> Signed-off-by: Maciej Rabeda <maciej.rabeda at linux.intel.com>
> Tested-by: Laszlo Ersek <lersek at redhat.com>
> ---
> NetworkPkg/Ip6Dxe/Ip6Nd.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Thanks for the patch. Two meta-suggestions:
(1) we should do one of the following:
(1a) Create a new BZ for this issue, cross-link it -- via the See Also
field -- with TianoCore#2174, and reference the new BZ in this commit
message. If we file this new BZ, it should get the Regression keyword.
(1b) Or else we should reopen TianoCore#2174, and reference *that* BZ in
this commit message.
(2) Independently of (1), the commit message should carry the following tag:
Fixes: 9c20342eed70ec99ec50cd73cb81804299f05403
Regarding this patch, the above updates only affect the commit message,
so I'm not asking for a v2 -- you can implement the commit message
changes right before pushing.
Thanks!
Laszlo
> 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
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56843): https://edk2.groups.io/g/devel/message/56843
Mute This Topic: https://groups.io/mt/72696739/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list