[ovs-dev] [PATCH] datapath-windows: Add validations for IP_HEADER_LEN

Shashank Ram rams at vmware.com
Thu Jun 15 22:15:47 UTC 2017


Adds validations in OvsGetIp() to make sure the IHL is
within valid bounds. If IHL is invalid, then the packet
is dropped by the callers of this function.

Signed-off-by: Shashank Ram <rams at vmware.com>
---
 datapath-windows/ovsext/Flow.c         | 5 +++++
 datapath-windows/ovsext/Offload.c      | 3 +++
 datapath-windows/ovsext/PacketParser.h | 9 ++++++++-
 datapath-windows/ovsext/Stt.c          | 2 +-
 datapath-windows/ovsext/User.c         | 5 +++++
 datapath-windows/ovsext/Vxlan.c        | 3 ++-
 6 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 60f9b1c..852a22f 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -2141,6 +2141,9 @@ OvsExtractLayers(const NET_BUFFER_LIST *packet,
                     }
                 }
             }
+        } else {
+            /* Invalid network header */
+            return NDIS_STATUS_INVALID_PACKET;
         }
     } else if (dlType == htons(ETH_TYPE_IPV6)) {
         NDIS_STATUS status;
@@ -2360,8 +2363,10 @@ OvsExtractFlow(const NET_BUFFER_LIST *packet,
                 }
             }
         } else {
+            /* Invalid network header */
             ((UINT64 *)ipKey)[0] = 0;
             ((UINT64 *)ipKey)[1] = 0;
+            return NDIS_STATUS_INVALID_PACKET;
         }
     } else if (flow->l2.dlType == htons(ETH_TYPE_IPV6)) {
         NDIS_STATUS status;
diff --git a/datapath-windows/ovsext/Offload.c b/datapath-windows/ovsext/Offload.c
index 65d3b67..0905c80 100644
--- a/datapath-windows/ovsext/Offload.c
+++ b/datapath-windows/ovsext/Offload.c
@@ -563,6 +563,9 @@ OvsValidateIPChecksum(PNET_BUFFER_LIST curNbl,
             if (checksum != hdrChecksum) {
                 return NDIS_STATUS_FAILURE;
             }
+        } else {
+            /* Invalid network header */
+            return NDIS_STATUS_FAILURE;
         }
     }
     return NDIS_STATUS_SUCCESS;
diff --git a/datapath-windows/ovsext/PacketParser.h b/datapath-windows/ovsext/PacketParser.h
index f1d7f28..0d5c0a6 100644
--- a/datapath-windows/ovsext/PacketParser.h
+++ b/datapath-windows/ovsext/PacketParser.h
@@ -17,6 +17,8 @@
 #ifndef __PACKET_PARSER_H_
 #define __PACKET_PARSER_H_ 1
 
+#define MIN_IPV4_HLEN 20
+
 #include "precomp.h"
 #include "NetProto.h"
 
@@ -107,7 +109,12 @@ OvsGetIp(const NET_BUFFER_LIST *packet,
     const IPHdr *ip = OvsGetPacketBytes(packet, sizeof *ip, ofs, storage);
     if (ip) {
         int ipLen = ip->ihl * 4;
-        if (ipLen >= sizeof *ip && OvsPacketLenNBL(packet) >= ofs + ipLen) {
+        if (ipLen <  MIN_IPV4_HLEN ||
+                ipLen > MAX_IPV4_HLEN ||
+                OvsPacketLenNBL(packet) < ofs + ipLen) {
+           /* IP header is invalid, flag it */
+           return NULL;
+        } else {
             return ip;
         }
     }
diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
index 1f36835..676cf0c 100644
--- a/datapath-windows/ovsext/Stt.c
+++ b/datapath-windows/ovsext/Stt.c
@@ -1019,7 +1019,7 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
                 innerIpHdr->check = IPChecksum((UINT8 *)innerIpHdr,
                                                 innerIpHdr->ihl * 4, 0);
             } else {
-                status = NDIS_STATUS_RESOURCES;
+                status = NDIS_STATUS_INVALID_PACKET;
                 goto dropNbl;
             }
         } else if (layers.isIPv6) {
diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c
index 7880220..22ee7af 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -465,6 +465,11 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
     ndisStatus = OvsExtractFlow(pNbl, execute->inPort, &key, &layers,
                      tempTunKey.tunKey.dst == 0 ? NULL : &tempTunKey.tunKey);
 
+    if (ndisStatus != NDIS_STATUS_SUCCESS) {
+        /* Invalid network header */
+        goto dropit;
+    }
+
     ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(pNbl);
     ctx->mru = execute->mru;
 
diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c
index 427f31c..f66a7e5 100644
--- a/datapath-windows/ovsext/Vxlan.c
+++ b/datapath-windows/ovsext/Vxlan.c
@@ -489,7 +489,8 @@ OvsSlowPathDecapVxlan(const PNET_BUFFER_LIST packet,
         if (nh) {
             layers.l4Offset = layers.l3Offset + nh->ihl * 4;
         } else {
-            break;
+           status = NDIS_STATUS_INVALID_PACKET;
+           break;
         }
 
         /* make sure it's a VXLAN packet */
-- 
2.9.3.windows.2



More information about the dev mailing list