diff mbox series

[ovs-dev,v2,1/7] datapath-windows: Use only non executable memory

Message ID 20171106153339.10100-1-aserdean@ovn.org
State Accepted
Headers show
Series [ovs-dev,v2,1/7] datapath-windows: Use only non executable memory | expand

Commit Message

Alin-Gabriel Serdean Nov. 6, 2017, 3:33 p.m. UTC 
From: Alin Serdean <aserdean@cloudbasesolutions.com>

Use only non-executable memory when using MmGetSystemAddressForMdlSafe.

Introduce a new function called OvsGetMdlWithLowPriority for readability.

Found using WDK 10 static code analysis.

Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
---
v2: Change OvsGetMdlWithLowPrio to OvsGetMdlWithLowPriority.
Suggested by: Shashank Ram <rams@vmware.com>
---
 datapath-windows/ovsext/BufferMgmt.c   |  8 ++++----
 datapath-windows/ovsext/Datapath.c     |  3 +--
 datapath-windows/ovsext/Flow.c         |  2 +-
 datapath-windows/ovsext/Geneve.c       |  5 ++---
 datapath-windows/ovsext/Gre.c          |  3 +--
 datapath-windows/ovsext/Offload.c      |  9 ++++-----
 datapath-windows/ovsext/PacketParser.c |  3 +--
 datapath-windows/ovsext/Stt.c          |  8 +++-----
 datapath-windows/ovsext/Util.h         | 15 +++++++++++++++
 datapath-windows/ovsext/Vxlan.c        |  7 +++----
 10 files changed, 35 insertions(+), 28 deletions(-)

Comments

Shashank Ram Nov. 7, 2017, 1:04 a.m. UTC | #1 
On Mon, Nov 6, 2017 at 7:33 AM, Alin Gabriel Serdean <aserdean@ovn.org>
wrote:

> From: Alin Serdean <aserdean@cloudbasesolutions.com>
>
> Use only non-executable memory when using MmGetSystemAddressForMdlSafe.
>
> Introduce a new function called OvsGetMdlWithLowPriority for readability.
>
> Found using WDK 10 static code analysis.
>
> Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
> ---
> v2: Change OvsGetMdlWithLowPrio to OvsGetMdlWithLowPriority.
> Suggested by: Shashank Ram <rams@vmware.com>
> ---
>
Acked-by: Shashank Ram <shashank08@gmail.com>
Alin Serdean Nov. 29, 2017, 2:56 p.m. UTC | #2 
Thanks for the reviews Shashank!

 

I applied the series on master.

 

From: Shashank Ram [mailto:shashank08@gmail.com] 
Sent: Tuesday, November 7, 2017 3:04 AM
To: Alin Gabriel Serdean <aserdean@ovn.org>
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v2 1/7] datapath-windows: Use only non executable memory

 

 

 

On Mon, Nov 6, 2017 at 7:33 AM, Alin Gabriel Serdean <aserdean@ovn.org <mailto:aserdean@ovn.org> > wrote:

From: Alin Serdean <aserdean@cloudbasesolutions.com <mailto:aserdean@cloudbasesolutions.com> >

Use only non-executable memory when using MmGetSystemAddressForMdlSafe.

Introduce a new function called OvsGetMdlWithLowPriority for readability.

Found using WDK 10 static code analysis.

Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com <mailto:aserdean@cloudbasesolutions.com> >
---
v2: Change OvsGetMdlWithLowPrio to OvsGetMdlWithLowPriority.
Suggested by: Shashank Ram <rams@vmware.com <mailto:rams@vmware.com> >
---

Acked-by: Shashank Ram <shashank08@gmail.com <mailto:shashank08@gmail.com> >
diff mbox series

Patch

diff --git a/datapath-windows/ovsext/BufferMgmt.c b/datapath-windows/ovsext/BufferMgmt.c
index ff4f9bb..03470d7 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -1157,7 +1157,7 @@  FixFragmentHeader(PNET_BUFFER nb, UINT16 fragmentSize,
 
     mdl = NET_BUFFER_FIRST_MDL(nb);
 
-    bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(mdl, LowPagePriority);
+    bufferStart = (PUINT8)OvsGetMdlWithLowPriority(mdl);
     if (!bufferStart) {
         return NDIS_STATUS_RESOURCES;
     }
@@ -1215,7 +1215,7 @@  FixSegmentHeader(PNET_BUFFER nb, UINT16 segmentSize, UINT32 seqNumber,
 
     mdl = NET_BUFFER_FIRST_MDL(nb);
 
-    bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(mdl, LowPagePriority);
+    bufferStart = (PUINT8)OvsGetMdlWithLowPriority(mdl);
     if (!bufferStart) {
         return NDIS_STATUS_RESOURCES;
     }
@@ -1521,8 +1521,8 @@  OvsAllocateNBLFromBuffer(PVOID context,
 
     nb = NET_BUFFER_LIST_FIRST_NB(nbl);
     mdl = NET_BUFFER_CURRENT_MDL(nb);
-    data = (PUINT8)MmGetSystemAddressForMdlSafe(mdl, LowPagePriority) +
-                    NET_BUFFER_CURRENT_MDL_OFFSET(nb);
+    data = (PUINT8)OvsGetMdlWithLowPriority(mdl)
+        + NET_BUFFER_CURRENT_MDL_OFFSET(nb);
     if (!data) {
         OvsCompleteNBL(switchContext, nbl, TRUE);
         return NULL;
diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index c68da51..34ef2b4 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -1593,8 +1593,7 @@  MapIrpOutputBuffer(PIRP irp,
     if (irp->MdlAddress == NULL) {
         return STATUS_INVALID_PARAMETER;
     }
-    *buffer = MmGetSystemAddressForMdlSafe(irp->MdlAddress,
-                                           NormalPagePriority);
+    *buffer = OvsGetMdlWithLowPriority(irp->MdlAddress);
     if (*buffer == NULL) {
         return STATUS_INSUFFICIENT_RESOURCES;
     }
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 852a22f..576fe4b 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -1933,7 +1933,7 @@  GetStartAddrNBL(const NET_BUFFER_LIST *_pNB)
 
     // Ethernet Header is a guaranteed safe access.
     curMdl = (NET_BUFFER_LIST_FIRST_NB(_pNB))->CurrentMdl;
-    curBuffer =  MmGetSystemAddressForMdlSafe(curMdl, LowPagePriority);
+    curBuffer = OvsGetMdlWithLowPriority(curMdl);
     if (!curBuffer) {
         return NULL;
     }
diff --git a/datapath-windows/ovsext/Geneve.c b/datapath-windows/ovsext/Geneve.c
index 6dca69b..821d5b8 100644
--- a/datapath-windows/ovsext/Geneve.c
+++ b/datapath-windows/ovsext/Geneve.c
@@ -157,8 +157,7 @@  NDIS_STATUS OvsEncapGeneve(POVS_VPORT_ENTRY vport,
         }
 
         curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-        bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
-                                                           LowPagePriority);
+        bufferStart = (PUINT8)OvsGetMdlWithLowPriority(curMdl);
         if (!bufferStart) {
             status = NDIS_STATUS_RESOURCES;
             goto ret_error;
@@ -286,7 +285,7 @@  NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT switchContext,
     curNbl = *newNbl;
     curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
     curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-    bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl, LowPagePriority)
+    bufferStart = (PUINT8)OvsGetMdlWithLowPriority(curMdl)
                   + NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
     if (!bufferStart) {
         status = NDIS_STATUS_RESOURCES;
diff --git a/datapath-windows/ovsext/Gre.c b/datapath-windows/ovsext/Gre.c
index f095742..1c8dbd2 100644
--- a/datapath-windows/ovsext/Gre.c
+++ b/datapath-windows/ovsext/Gre.c
@@ -202,8 +202,7 @@  OvsDoEncapGre(POVS_VPORT_ENTRY vport,
         }
 
         curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-        bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
-                                                           LowPagePriority);
+        bufferStart = (PUINT8)OvsGetMdlWithLowPriority(curMdl);
         if (!bufferStart) {
             status = NDIS_STATUS_RESOURCES;
             goto ret_error;
diff --git a/datapath-windows/ovsext/Offload.c b/datapath-windows/ovsext/Offload.c
index 0905c80..944d3b0 100644
--- a/datapath-windows/ovsext/Offload.c
+++ b/datapath-windows/ovsext/Offload.c
@@ -460,7 +460,7 @@  CalculateChecksumNB(const PNET_BUFFER nb,
 
     firstMdlLen = MIN(firstMdlLen, packetLen);
     if (offset < firstMdlLen) {
-        src = (PUCHAR) MmGetSystemAddressForMdlSafe(currentMdl, LowPagePriority);
+        src = (PUCHAR)OvsGetMdlWithLowPriority(currentMdl);
         if (!src) {
             return 0;
         }
@@ -485,7 +485,7 @@  CalculateChecksumNB(const PNET_BUFFER nb,
             mdlLen = MIN(mdlLen, packetLen);
         }
 
-        src = (PUCHAR)MmGetSystemAddressForMdlSafe(currentMdl, LowPagePriority);
+        src = (PUCHAR)OvsGetMdlWithLowPriority(currentMdl);
         if (!src) {
             return 0;
         }
@@ -504,7 +504,7 @@  CalculateChecksumNB(const PNET_BUFFER nb,
         csumDataLen -= csLen;
         currentMdl = NDIS_MDL_LINKAGE(currentMdl);
         if (csumDataLen && currentMdl) {
-            src = MmGetSystemAddressForMdlSafe(currentMdl, LowPagePriority);
+            src = OvsGetMdlWithLowPriority(currentMdl);
             if (!src) {
                 return 0;
             }
@@ -670,8 +670,7 @@  OvsApplySWChecksumOnNB(POVS_PACKET_HDR_INFO layers,
          curNb = curNb->Next) {
         packetLength = NET_BUFFER_DATA_LENGTH(curNb);
         curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-        bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
-                                                           LowPagePriority);
+        bufferStart = (PUINT8)OvsGetMdlWithLowPriority(curMdl);
         if (!bufferStart) {
             return NDIS_STATUS_RESOURCES;
         }
diff --git a/datapath-windows/ovsext/PacketParser.c b/datapath-windows/ovsext/PacketParser.c
index c4a04d0..f8adcc3 100644
--- a/datapath-windows/ovsext/PacketParser.c
+++ b/datapath-windows/ovsext/PacketParser.c
@@ -38,8 +38,7 @@  OvsGetPacketBytes(const NET_BUFFER_LIST *nbl,
 
     // Data on current MDL may be offset from start of MDL
     while (destOffset < copyLen && currentMdl) {
-        PUCHAR srcMemory = MmGetSystemAddressForMdlSafe(currentMdl,
-                                                        LowPagePriority);
+        PUCHAR srcMemory = OvsGetMdlWithLowPriority(currentMdl);
         ULONG length = MmGetMdlByteCount(currentMdl);
         if (!srcMemory) {
             status = NDIS_STATUS_RESOURCES;
diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
index f98070f..e2914e4 100644
--- a/datapath-windows/ovsext/Stt.c
+++ b/datapath-windows/ovsext/Stt.c
@@ -215,8 +215,7 @@  OvsDoEncapStt(POVS_VPORT_ENTRY vport,
             curNb = curNb->Next) {
         curMdl = NET_BUFFER_CURRENT_MDL(curNb);
         innerFrameLen = NET_BUFFER_DATA_LENGTH(curNb);
-        bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
-                                                           LowPagePriority);
+        bufferStart = (PUINT8)OvsGetMdlWithLowPriority(curMdl);
         if (bufferStart == NULL) {
             status = NDIS_STATUS_RESOURCES;
             goto ret_error;
@@ -266,7 +265,7 @@  OvsDoEncapStt(POVS_VPORT_ENTRY vport,
         ASSERT((int) (MmGetMdlByteCount(curMdl) -
                     NET_BUFFER_CURRENT_MDL_OFFSET(curNb)) >= (int) headRoom);
 
-        buf = (PUINT8) MmGetSystemAddressForMdlSafe(curMdl, LowPagePriority);
+        buf = (PUINT8)OvsGetMdlWithLowPriority(curMdl);
         if (!buf) {
             ASSERT(!"MmGetSystemAddressForMdlSafe failed");
             OVS_LOG_ERROR("MmGetSystemAddressForMdlSafe failed");
@@ -845,8 +844,7 @@  OvsDecapSetOffloads(PNET_BUFFER_LIST *curNbl,
         curNb = NET_BUFFER_LIST_FIRST_NB(*curNbl);
         curMdl = NET_BUFFER_CURRENT_MDL(curNb);
 
-        buf = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
-            LowPagePriority);
+        buf = (PUINT8)OvsGetMdlWithLowPriority(curMdl);
         if (buf == NULL) {
             return NDIS_STATUS_RESOURCES;
         }
diff --git a/datapath-windows/ovsext/Util.h b/datapath-windows/ovsext/Util.h
index f73c71f..6f02147 100644
--- a/datapath-windows/ovsext/Util.h
+++ b/datapath-windows/ovsext/Util.h
@@ -152,4 +152,19 @@  UINT32 Rand()
     return seed.LowPart *= 0x8088405 + 1;
 }
 
+/*
+ *----------------------------------------------------------------------------
+ *  OvsGetMdlWithLowPriority --
+ *    Return the nonpaged system-space virtual address for the given MDL
+ *    `curMdl` using low page priority and no executable memory.
+ *----------------------------------------------------------------------------
+ */
+
+static __inline
+PVOID OvsGetMdlWithLowPriority(PMDL curMdl)
+{
+return	MmGetSystemAddressForMdlSafe(curMdl,
+                                     LowPagePriority | MdlMappingNoExecute);
+}
+
 #endif /* __UTIL_H_ */
diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c
index f66a7e5..f6277f5 100644
--- a/datapath-windows/ovsext/Vxlan.c
+++ b/datapath-windows/ovsext/Vxlan.c
@@ -244,8 +244,7 @@  OvsDoEncapVxlan(POVS_VPORT_ENTRY vport,
         }
 
         curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-        bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl,
-                                                           LowPagePriority);
+        bufferStart = (PUINT8)OvsGetMdlWithLowPriority(curMdl);
         if (!bufferStart) {
             status = NDIS_STATUS_RESOURCES;
             goto ret_error;
@@ -415,8 +414,8 @@  OvsDecapVxlan(POVS_SWITCH_CONTEXT switchContext,
     curNbl = *newNbl;
     curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
     curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-    bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl, LowPagePriority) +
-                  NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
+    bufferStart = (PUINT8)OvsGetMdlWithLowPriority(curMdl)
+        + NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
     if (!bufferStart) {
         status = NDIS_STATUS_RESOURCES;
         goto dropNbl;