Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP2:GA
ovmf.16349
ovmf-bsc1163927-fix-ping-and-ip6dxe.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File ovmf-bsc1163927-fix-ping-and-ip6dxe.patch of Package ovmf.16349
From f6329329cf951847c0d8dfaf3a817b31696423b6 Mon Sep 17 00:00:00 2001 From: Maciej Rabeda <maciej.rabeda@linux.intel.com> Date: Thu, 27 Feb 2020 11:30:43 +0100 Subject: [PATCH 1/4] ShellPkg: Fix 'ping' command Ip4 receive flow. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2032 'ping' command's receive flow utilizes a single Rx token which it attempts to reuse before recycling the previously received packet. This causes a situation where under ICMP traffic, Ping6OnEchoReplyReceived() function will receive an already recycled packet with EFI_SUCCESS token status and finally dereference invalid pointers from RxData structure. Cc: Ray Ni <ray.ni@intel.com> Cc: Zhichao Gao <zhichao.gao@intel.com> Signed-off-by: Maciej Rabeda <maciej.rabeda@linux.intel.com> Reviewed-by: Siyuan Fu <siyuan.fu@intel.com> Acked-by: Zhichao Gao <zhichao.gao@intel.com> (cherry picked from commit 65c73df44c61235ede84c5aa1d2eab6650844966) --- ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c index 10d291c31446..b12b7bc8052b 100644 --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c @@ -620,6 +620,11 @@ Ping6OnEchoReplyReceived ( ON_EXIT: + // + // Recycle the packet before reusing RxToken + // + gBS->SignalEvent (Private->IpChoice == PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal); + if (Private->RxCount < Private->SendNum) { // // Continue to receive icmp echo reply packets. @@ -638,10 +643,6 @@ ON_EXIT: // Private->Status = EFI_SUCCESS; } - // - // Singal to recycle the each rxdata here, not at the end of process. - // - gBS->SignalEvent (Private->IpChoice == PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal); } /** -- 2.25.1 From 209c8bb34ee5d062c1a86e67c3e4bac845a65264 Mon Sep 17 00:00:00 2001 From: "Vitaly Cheptsov via Groups.Io" <vit9696=protonmail.com@groups.io> Date: Sat, 17 Aug 2019 07:28:14 +0800 Subject: [PATCH 2/4] MdePkg: Add STATIC_ASSERT macro REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2048 Provide a macro for compile time assertions. Equivalent to C11 static_assert macro from assert.h. Signed-off-by: Vitaly Cheptsov <vit9696@protonmail.com> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com> Reviewed-by: Michael Kinney <michael.d.kinney@intel.com> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com> Acked-by: Jiewen Yao <jiewen.yao@intel.com> (cherry picked from commit 204ae9da230ecbf0910c21acac7aa5d5e8cbb8d0) --- MdePkg/Include/Base.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h index 02140a5ac2ee..52252e7dc500 100644 --- a/MdePkg/Include/Base.h +++ b/MdePkg/Include/Base.h @@ -801,6 +801,20 @@ typedef UINTN *BASE_LIST; #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field)) #endif +/** + Portable definition for compile time assertions. + Equivalent to C11 static_assert macro from assert.h. + + @param Expression Boolean expression. + @param Message Raised compiler diagnostic message when expression is false. + +**/ +#ifdef _MSC_EXTENSIONS + #define STATIC_ASSERT static_assert +#else + #define STATIC_ASSERT _Static_assert +#endif + /** Macro that returns a pointer to the data structure that contains a specified field of that data structure. This is a lightweight method to hide information by placing a -- 2.25.1 From 33557e0062181b44ebc695c0776f971d2fcab3e2 Mon Sep 17 00:00:00 2001 From: Maciej Rabeda <maciej.rabeda@linux.intel.com> Date: Mon, 2 Mar 2020 13:25:20 +0100 Subject: [PATCH 3/4] NetworkPkg/Ip6Dxe: Improve Neightbor Discovery message validation. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2174 Problem has been identified with Ip6ProcessRouterAdvertise() when Router Advertise packet contains options with malicious/invalid 'Length' field. This can lead to platform entering infinite loop when processing options from that packet. 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> (cherry picked from commit 9c20342eed70ec99ec50cd73cb81804299f05403) --- NetworkPkg/Ip6Dxe/Ip6Nd.c | 44 ++++++++++++++++----------- NetworkPkg/Ip6Dxe/Ip6Nd.h | 13 ++++++++ NetworkPkg/Ip6Dxe/Ip6Option.c | 57 ++++++++++++++++++++++++++--------- 3 files changed, 83 insertions(+), 31 deletions(-) diff --git a/NetworkPkg/Ip6Dxe/Ip6Nd.c b/NetworkPkg/Ip6Dxe/Ip6Nd.c index a3f49bb2daad..3cb0a70b6ee2 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Nd.c +++ b/NetworkPkg/Ip6Dxe/Ip6Nd.c @@ -1933,7 +1933,7 @@ Ip6ProcessRouterAdvertise ( UINT32 ReachableTime; UINT32 RetransTimer; UINT16 RouterLifetime; - UINT16 Offset; + UINT32 Offset; UINT8 Type; UINT8 Length; IP6_ETHER_ADDR_OPTION LinkLayerOption; @@ -2100,10 +2100,11 @@ Ip6ProcessRouterAdvertise ( // // The only defined options that may appear are the Source // Link-Layer Address, Prefix information and MTU options. - // All included options have a length that is greater than zero. + // All included options have a length that is greater than zero and + // fit within the input packet. // Offset = 16; - while (Offset < Head->PayloadLength) { + while (Offset < (UINT32) Head->PayloadLength) { NetbufCopy (Packet, Offset, sizeof (UINT8), &Type); switch (Type) { case Ip6OptionEtherSource: @@ -2111,9 +2112,12 @@ Ip6ProcessRouterAdvertise ( // Update the neighbor cache // NetbufCopy (Packet, Offset, sizeof (IP6_ETHER_ADDR_OPTION), (UINT8 *) &LinkLayerOption); - if (LinkLayerOption.Length <= 0) { - goto Exit; - } + + // + // Option size validity ensured by Ip6IsNDOptionValid(). + // + ASSERT (LinkLayerOption.Length != 0); + ASSERT (Offset + (UINT32) LinkLayerOption.Length * 8 >= (UINT32) Head->PayloadLength); ZeroMem (&LinkLayerAddress, sizeof (EFI_MAC_ADDRESS)); CopyMem (&LinkLayerAddress, LinkLayerOption.EtherAddr, 6); @@ -2157,13 +2161,17 @@ Ip6ProcessRouterAdvertise ( } } - Offset = (UINT16) (Offset + (UINT16) LinkLayerOption.Length * 8); + Offset += (UINT32) LinkLayerOption.Length * 8; break; case Ip6OptionPrefixInfo: NetbufCopy (Packet, Offset, sizeof (IP6_PREFIX_INFO_OPTION), (UINT8 *) &PrefixOption); - if (PrefixOption.Length != 4) { - goto Exit; - } + + // + // Option size validity ensured by Ip6IsNDOptionValid(). + // + ASSERT (PrefixOption.Length == 4); + ASSERT (Offset + (UINT32) PrefixOption.Length * 8 >= (UINT32) Head->PayloadLength); + PrefixOption.ValidLifetime = NTOHL (PrefixOption.ValidLifetime); PrefixOption.PreferredLifetime = NTOHL (PrefixOption.PreferredLifetime); @@ -2327,9 +2335,12 @@ Ip6ProcessRouterAdvertise ( break; case Ip6OptionMtu: NetbufCopy (Packet, Offset, sizeof (IP6_MTU_OPTION), (UINT8 *) &MTUOption); - if (MTUOption.Length != 1) { - goto Exit; - } + + // + // Option size validity ensured by Ip6IsNDOptionValid(). + // + ASSERT (MTUOption.Length == 1); + ASSERT (Offset + (UINT32) MTUOption.Length * 8 >= (UINT32) Head->PayloadLength); // // Use IPv6 minimum link MTU 1280 bytes as the maximum packet size in order @@ -2344,11 +2355,10 @@ Ip6ProcessRouterAdvertise ( // Silently ignore unrecognized options // NetbufCopy (Packet, Offset + sizeof (UINT8), sizeof (UINT8), &Length); - if (Length <= 0) { - goto Exit; - } - Offset = (UINT16) (Offset + (UINT16) Length * 8); + ASSERT (Length != 0); + + Offset += (UINT32) Length * 8; break; } } diff --git a/NetworkPkg/Ip6Dxe/Ip6Nd.h b/NetworkPkg/Ip6Dxe/Ip6Nd.h index 982203ca5ffc..4d10baf6af25 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Nd.h +++ b/NetworkPkg/Ip6Dxe/Ip6Nd.h @@ -62,12 +62,21 @@ VOID VOID *Context ); +typedef struct _IP6_OPTION_HEADER { + UINT8 Type; + UINT8 Length; +} IP6_OPTION_HEADER; + +STATIC_ASSERT (sizeof (IP6_OPTION_HEADER) == 2, "IP6_OPTION_HEADER is expected to be exactly 2 bytes long."); + typedef struct _IP6_ETHE_ADDR_OPTION { UINT8 Type; UINT8 Length; UINT8 EtherAddr[6]; } IP6_ETHER_ADDR_OPTION; +STATIC_ASSERT (sizeof (IP6_ETHER_ADDR_OPTION) == 8, "IP6_ETHER_ADDR_OPTION is expected to be exactly 8 bytes long."); + typedef struct _IP6_MTU_OPTION { UINT8 Type; UINT8 Length; @@ -75,6 +84,8 @@ typedef struct _IP6_MTU_OPTION { UINT32 Mtu; } IP6_MTU_OPTION; +STATIC_ASSERT (sizeof (IP6_MTU_OPTION) == 8, "IP6_MTU_OPTION is expected to be exactly 8 bytes long."); + typedef struct _IP6_PREFIX_INFO_OPTION { UINT8 Type; UINT8 Length; @@ -86,6 +97,8 @@ typedef struct _IP6_PREFIX_INFO_OPTION { EFI_IPv6_ADDRESS Prefix; } IP6_PREFIX_INFO_OPTION; +STATIC_ASSERT (sizeof (IP6_PREFIX_INFO_OPTION) == 32, "IP6_PREFIX_INFO_OPTION is expected to be exactly 32 bytes long."); + typedef VOID (*IP6_DAD_CALLBACK) ( diff --git a/NetworkPkg/Ip6Dxe/Ip6Option.c b/NetworkPkg/Ip6Dxe/Ip6Option.c index 21d28b6942a1..43c67fdc43a2 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Option.c +++ b/NetworkPkg/Ip6Dxe/Ip6Option.c @@ -135,45 +135,74 @@ Ip6IsNDOptionValid ( IN UINT16 OptionLen ) { - UINT16 Offset; - UINT8 OptionType; + UINT32 Offset; UINT16 Length; + IP6_OPTION_HEADER *OptionHeader; + + if (Option == NULL) { + ASSERT (Option != NULL); + return FALSE; + } Offset = 0; - while (Offset < OptionLen) { - OptionType = *(Option + Offset); - Length = (UINT16) (*(Option + Offset + 1) * 8); + // + // RFC 4861 states that Neighbor Discovery packet can contain zero or more + // options. Start processing the options if at least Type + Length fields + // fit within the input buffer. + // + while (Offset + sizeof (IP6_OPTION_HEADER) - 1 < OptionLen) { + OptionHeader = (IP6_OPTION_HEADER*) (Option + Offset); + Length = (UINT16) OptionHeader->Length * 8; - switch (OptionType) { + switch (OptionHeader->Type) { case Ip6OptionPrefixInfo: if (Length != 32) { return FALSE; } - break; case Ip6OptionMtu: if (Length != 8) { return FALSE; } - break; default: - // - // Check the length of Ip6OptionEtherSource, Ip6OptionEtherTarget, and - // Ip6OptionRedirected here. For unrecognized options, silently ignore - // and continue processsing the message. - // + // RFC 4861 states that Length field cannot be 0. if (Length == 0) { return FALSE; } + break; + } + + // + // Check whether recognized options are within the input buffer's scope. + // + switch (OptionHeader->Type) { + case Ip6OptionEtherSource: + case Ip6OptionEtherTarget: + case Ip6OptionPrefixInfo: + case Ip6OptionRedirected: + case Ip6OptionMtu: + if (Offset + Length > (UINT32) OptionLen) { + return FALSE; + } + break; + default: + // + // Unrecognized options can be either valid (but unused) or invalid + // (garbage in between or right after valid options). Silently ignore. + // break; } - Offset = (UINT16) (Offset + Length); + // + // Advance to the next option. + // Length already considers option header's Type + Length. + // + Offset += Length; } return TRUE; -- 2.25.1 From eed0574a948c5c052529c4217f53c104fb95216b Mon Sep 17 00:00:00 2001 From: Maciej Rabeda <maciej.rabeda@linux.intel.com> Date: Wed, 1 Apr 2020 11:43:55 +0200 Subject: [PATCH 4/4] 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 (cherry picked from commit 4deef2d865efdc61d1a53ad7bd48f9dd42560b45) --- 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 3cb0a70b6ee2..bcb15e5fe428 100644 --- a/NetworkPkg/Ip6Dxe/Ip6Nd.c +++ b/NetworkPkg/Ip6Dxe/Ip6Nd.c @@ -2117,7 +2117,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); @@ -2170,7 +2170,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); @@ -2340,7 +2340,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.25.1
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
