From 9323fd92727eed35401b9997237e35fb4aa03f36 Mon Sep 17 00:00:00 2001
From: Ilya Maximets <i.maximets@ovn.org>
Date: Thu, 6 Jun 2024 12:52:33 +0200
Subject: [PATCH] datapath-windows: Fix parsing of split buffers in
 OvsGetTcpHeader.

NdisGetDataBuffer() is called without providing a buffer to copy packet
data in case it is not contiguous.  So, it fails in some scenarios
where the packet is handled by the general network stack before OVS
and headers become split in multiple buffers.

Use existing helpers to retrieve the headers instead, they are using
OvsGetPacketBytes() which should be able to handle split data.

It might be a bit slower than getting direct pointers that may be
provided by NdisGetDataBuffer(), but it's better to optimize commonly
used OvsGetPacketBytes() helper in the future instead of optimizing
every single caller separately.  And we're still copying the TCP
header anyway.

Fixes: 9726a016d9d6 ("datapath-windows: Implement locking in conntrack NAT.")
Reported-at: https://github.com/openvswitch/ovs-issues/issues/323
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: 0-day Robot <robot@bytheb.org>
---
 datapath-windows/ovsext/Conntrack.c | 45 ++++++++++++++---------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
index 39ba5cc10ec..4649805dd9c 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -678,46 +678,43 @@ OvsGetTcpHeader(PNET_BUFFER_LIST nbl,
                 VOID *storage,
                 UINT32 *tcpPayloadLen)
 {
-    IPHdr *ipHdr;
-    IPv6Hdr *ipv6Hdr;
-    TCPHdr *tcp;
+    IPv6Hdr ipv6HdrStorage;
+    IPHdr ipHdrStorage;
+    const IPv6Hdr *ipv6Hdr;
+    const IPHdr *ipHdr;
+    const TCPHdr *tcp;
     VOID *dest = storage;
     uint16_t ipv6ExtLength = 0;
 
     if (layers->isIPv6) {
-        ipv6Hdr = NdisGetDataBuffer(NET_BUFFER_LIST_FIRST_NB(nbl),
-                                    layers->l4Offset + sizeof(TCPHdr),
-                                    NULL, 1, 0);
+        ipv6Hdr = OvsGetPacketBytes(nbl, sizeof *ipv6Hdr,
+                                    layers->l3Offset, &ipv6HdrStorage);
         if (ipv6Hdr == NULL) {
             return NULL;
         }
 
-        tcp = (TCPHdr *)((PCHAR)ipv6Hdr + layers->l4Offset);
-        ipv6Hdr = (IPv6Hdr *)((PCHAR)ipv6Hdr + layers->l3Offset);
-        if (tcp->doff * 4 >= sizeof *tcp) {
-            NdisMoveMemory(dest, tcp, sizeof(TCPHdr));
-            ipv6ExtLength = layers->l4Offset - layers->l3Offset - sizeof(IPv6Hdr);
-            *tcpPayloadLen = (ntohs(ipv6Hdr->payload_len) - ipv6ExtLength - TCP_HDR_LEN(tcp));
-            return storage;
+        tcp = OvsGetTcp(nbl, layers->l4Offset, dest);
+        if (tcp == NULL) {
+            return NULL;
         }
+
+        ipv6ExtLength = layers->l4Offset - layers->l3Offset - sizeof(IPv6Hdr);
+        *tcpPayloadLen = (ntohs(ipv6Hdr->payload_len) - ipv6ExtLength - TCP_HDR_LEN(tcp));
     } else {
-        ipHdr = NdisGetDataBuffer(NET_BUFFER_LIST_FIRST_NB(nbl),
-                                  layers->l4Offset + sizeof(TCPHdr),
-                                  NULL, 1 /*no align*/, 0);
+        ipHdr = OvsGetIp(nbl, layers->l3Offset, &ipHdrStorage);
         if (ipHdr == NULL) {
             return NULL;
         }
 
-        ipHdr = (IPHdr *)((PCHAR)ipHdr + layers->l3Offset);
-        tcp = (TCPHdr *)((PCHAR)ipHdr + ipHdr->ihl * 4);
-
-        if (tcp->doff * 4 >= sizeof *tcp) {
-            NdisMoveMemory(dest, tcp, sizeof(TCPHdr));
-            *tcpPayloadLen = TCP_DATA_LENGTH(ipHdr, tcp);
-            return storage;
+        tcp = OvsGetTcp(nbl, layers->l4Offset, dest);
+        if (tcp == NULL) {
+            return NULL;
         }
+
+        *tcpPayloadLen = TCP_DATA_LENGTH(ipHdr, tcp);
     }
-    return NULL;
+
+    return storage;
 }
 
 static UINT8