File ovmf-bsc1163927-fix-ping-and-ip6dxe.patch of Package ovmf

Overview Repositories Revisions Requests Users Attributes Meta

File ovmf-bsc1163927-fix-ping-and-ip6dxe.patch of Package ovmf (Revision 501d9b877cec06be046b039fc89db796)

Currently displaying revision 501d9b877cec06be046b039fc89db796 , Show latest

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

Places

File ovmf-bsc1163927-fix-ping-and-ip6dxe.patch of Package ovmf (Revision 501d9b877cec06be046b039fc89db796)

Places