Comments inline. Thanks, Shashank
________________________________________ From: ovs-dev-boun...@openvswitch.org <ovs-dev-boun...@openvswitch.org> on behalf of Alin Serdean <aserd...@cloudbasesolutions.com> Sent: Thursday, July 13, 2017 9:40 PM To: d...@openvswitch.org Subject: [ovs-dev] [PATCH 01/40] datapath-windows: Use only non executable memory Use only non-executable memory when using MmGetSystemAddressForMdlSafe. Introduce a new function called OvsGetMdlWithLowPrio for readability. Found using WDK 10 static code analysis. Signed-off-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.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(-) diff --git a/datapath-windows/ovsext/BufferMgmt.c b/datapath-windows/ovsext/BufferMgmt.c index 5048ada..6354781 100644 --- a/datapath-windows/ovsext/BufferMgmt.c +++ b/datapath-windows/ovsext/BufferMgmt.c @@ -1153,7 +1153,7 @@ FixFragmentHeader(PNET_BUFFER nb, UINT16 fragmentSize, mdl = NET_BUFFER_FIRST_MDL(nb); - bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(mdl, LowPagePriority); + bufferStart = (PUINT8)OvsGetMdlWithLowPrio(mdl); if (!bufferStart) { return NDIS_STATUS_RESOURCES; } @@ -1211,7 +1211,7 @@ FixSegmentHeader(PNET_BUFFER nb, UINT16 segmentSize, UINT32 seqNumber, mdl = NET_BUFFER_FIRST_MDL(nb); - bufferStart = (PUINT8)MmGetSystemAddressForMdlSafe(mdl, LowPagePriority); + bufferStart = (PUINT8)OvsGetMdlWithLowPrio(mdl); if (!bufferStart) { return NDIS_STATUS_RESOURCES; } @@ -1517,8 +1517,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)OvsGetMdlWithLowPrio(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 10412a1..21693af 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -1588,8 +1588,7 @@ MapIrpOutputBuffer(PIRP irp, if (irp->MdlAddress == NULL) { return STATUS_INVALID_PARAMETER; } - *buffer = MmGetSystemAddressForMdlSafe(irp->MdlAddress, - NormalPagePriority); + *buffer = OvsGetMdlWithLowPrio(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..a7d6317 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 = OvsGetMdlWithLowPrio(curMdl); if (!curBuffer) { return NULL; } diff --git a/datapath-windows/ovsext/Geneve.c b/datapath-windows/ovsext/Geneve.c index 43374e2..01b5416 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)OvsGetMdlWithLowPrio(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)OvsGetMdlWithLowPrio(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..461efe0 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)OvsGetMdlWithLowPrio(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..6cc000b 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)OvsGetMdlWithLowPrio(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)OvsGetMdlWithLowPrio(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 = OvsGetMdlWithLowPrio(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)OvsGetMdlWithLowPrio(curMdl); if (!bufferStart) { return NDIS_STATUS_RESOURCES; } diff --git a/datapath-windows/ovsext/PacketParser.c b/datapath-windows/ovsext/PacketParser.c index c4a04d0..a52e713 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 = OvsGetMdlWithLowPrio(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 676cf0c..5490062 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)OvsGetMdlWithLowPrio(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)OvsGetMdlWithLowPrio(curMdl); if (!buf) { ASSERT(!"MmGetSystemAddressForMdlSafe failed"); OVS_LOG_ERROR("MmGetSystemAddressForMdlSafe failed"); @@ -839,8 +838,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)OvsGetMdlWithLowPrio(curMdl); if (buf == NULL) { return NDIS_STATUS_RESOURCES; } diff --git a/datapath-windows/ovsext/Util.h b/datapath-windows/ovsext/Util.h index 7aa91ec..cccf9ff 100644 --- a/datapath-windows/ovsext/Util.h +++ b/datapath-windows/ovsext/Util.h @@ -146,4 +146,19 @@ UINT32 Rand() return seed.LowPart *= 0x8088405 + 1; } +/* + *---------------------------------------------------------------------------- + * OvsGetMdlWithLowPrio -- + * Return the nonpaged system-space virtual address for the given MDL + * `curMdl` using low page priority and no executable memory. + *---------------------------------------------------------------------------- + */ + +static __inline +PVOID OvsGetMdlWithLowPrio(PMDL curMdl) >> Mind renaming this to OvsGetMdlWithLowPriority? Its more readable at the >> cost of 4 chars. +{ +return MmGetSystemAddressForMdlSafe(curMdl, + LowPagePriority | MdlMappingNoExecute); +} + #endif /* __UTIL_H_ */ diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c index f66a7e5..c4315fa 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)OvsGetMdlWithLowPrio(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)OvsGetMdlWithLowPrio(curMdl) + + NET_BUFFER_CURRENT_MDL_OFFSET(curNb); if (!bufferStart) { status = NDIS_STATUS_RESOURCES; goto dropNbl; -- 2.10.2.windows.1 _______________________________________________ _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev