Thanks for patching this.

Acked-by: Sairam Venugopal <[email protected]>





On 6/15/17, 3:15 PM, "[email protected] on behalf of Shashank 
Ram" <[email protected] on behalf of [email protected]> wrote:

>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 <[email protected]>
>---
> 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
>
>_______________________________________________
>dev mailing list
>[email protected]
>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=ig_cwF-VoHzn5YBfQ2Co4yRaOwQWzMmMGsiVexczTGU&s=tPTXxlUcTwT5ExbHfxCk_TRm5a90gp6lv_CO1FZ1xco&e=
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to