[ovs-dev] [PATCH V2] datapath-windows: Validate netlink packets integrity

Nithin Raju nithin at vmware.com
Thu May 19 22:30:36 UTC 2016


Hi Paul,
I looked at the change in detail and it is definitely in the right spirit
to harden the kernel datapath code.

However, I thought a few things could be simplified a little. I will be
sending out a couple of simple reviews on top of your patch (that is
already submitted). Pls. take a look.

Other than that the patch looks good.

Thanks,
-- Nithin

-----Original Message-----
From: dev <dev-bounces at openvswitch.org> on behalf of Paul Boca
<pboca at cloudbasesolutions.com>
Date: Wednesday, April 27, 2016 at 1:05 AM
To: "dev at openvswitch.org" <dev at openvswitch.org>
Subject: [ovs-dev] [PATCH V2] datapath-windows: Validate netlink
packets	integrity

>Solved access violation when trying to acces netling message - obtained
>with forged IOCTLs
>
>Signed-off-by: Paul-Daniel Boca <pboca at cloudbasesolutions.com>
>Acked-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
>---
>V2: Fixed alignement problems
>---
> datapath-windows/ovsext/Datapath.c        | 45 ++++++++++++++++++---
> datapath-windows/ovsext/Flow.c            | 42 +++++++++++---------
> datapath-windows/ovsext/Netlink/Netlink.c | 66
>++++++++++++++++++++++++++-----
> datapath-windows/ovsext/Netlink/Netlink.h | 13 ++++--
> datapath-windows/ovsext/User.c            |  5 ++-
> datapath-windows/ovsext/Vport.c           | 34 ++++++++--------
> lib/netlink-socket.c                      |  2 +
> 7 files changed, 152 insertions(+), 55 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Datapath.c
>b/datapath-windows/ovsext/Datapath.c
>index 06f99b3..1f89964 100644
>--- a/datapath-windows/ovsext/Datapath.c
>+++ b/datapath-windows/ovsext/Datapath.c
>@@ -307,6 +307,7 @@ static NTSTATUS MapIrpOutputBuffer(PIRP irp,
> static NTSTATUS ValidateNetlinkCmd(UINT32 devOp,
>                                    POVS_OPEN_INSTANCE instance,
>                                    POVS_MESSAGE ovsMsg,
>+                                   UINT32 ovsMgsLength,
>                                    NETLINK_FAMILY *nlFamilyOps);
> static NTSTATUS InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT
>usrParamsCtx,
>                                         NETLINK_FAMILY *nlFamilyOps,
>@@ -694,6 +695,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
>     UINT32 devOp;
>     OVS_MESSAGE ovsMsgReadOp;
>     POVS_MESSAGE ovsMsg;
>+    UINT32 ovsMsgLength = 0;
>     NETLINK_FAMILY *nlFamilyOps;
>     OVS_USER_PARAMS_CONTEXT usrParamsCtx;
> 
>@@ -774,6 +776,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
>         }
> 
>         ovsMsg = inputBuffer;
>+        ovsMsgLength = inputBufferLen;
>         devOp = OVS_TRANSACTION_DEV_OP;
>         break;
> 
>@@ -808,6 +811,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
>                               OVS_CTRL_CMD_EVENT_NOTIFY :
>                               OVS_CTRL_CMD_READ_NOTIFY;
>         ovsMsg->genlMsg.version = nlControlFamilyOps.version;
>+        ovsMsgLength = outputBufferLen;
> 
>         devOp = OVS_READ_DEV_OP;
>         break;
>@@ -853,6 +857,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
> 
>         /* Create an NL message for consumption. */
>         ovsMsg = &ovsMsgReadOp;
>+        ovsMsgLength = sizeof (ovsMsgReadOp);
>         devOp = OVS_READ_DEV_OP;
> 
>         break;
>@@ -864,7 +869,21 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
>             goto done;
>         }
> 
>+        /*
>+         * Output buffer not mandatory but map it in case we have
>something
>+         * to return to requester.
>+        */
>+        if (outputBufferLen != 0) {
>+            status = MapIrpOutputBuffer(irp, outputBufferLen,
>+                sizeof *ovsMsg, &outputBuffer);
>+            if (status != STATUS_SUCCESS) {
>+                goto done;
>+            }
>+            ASSERT(outputBuffer);
>+        }
>+
>         ovsMsg = inputBuffer;
>+        ovsMsgLength = inputBufferLen;
>         devOp = OVS_WRITE_DEV_OP;
>         break;
> 
>@@ -903,7 +922,8 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
>      * "artificial" or was copied from a previously validated 'ovsMsg'.
>      */
>     if (devOp != OVS_READ_DEV_OP) {
>-        status = ValidateNetlinkCmd(devOp, instance, ovsMsg,
>nlFamilyOps);
>+        status = ValidateNetlinkCmd(devOp, instance, ovsMsg,
>+                                    ovsMsgLength, nlFamilyOps);
>         if (status != STATUS_SUCCESS) {
>             goto done;
>         }
>@@ -938,11 +958,18 @@ static NTSTATUS
> ValidateNetlinkCmd(UINT32 devOp,
>                    POVS_OPEN_INSTANCE instance,
>                    POVS_MESSAGE ovsMsg,
>+                   UINT32 ovsMsgLength,
>                    NETLINK_FAMILY *nlFamilyOps)
> {
>     NTSTATUS status = STATUS_INVALID_PARAMETER;
>     UINT16 i;
> 
>+    // We need to ensure we have enough data to process
>+    if (NlMsgSize(&ovsMsg->nlMsg) > ovsMsgLength) {
>+        status = STATUS_INVALID_PARAMETER;
>+        goto done;
>+    }

Sure, this can be a useful check for integrity. We have not relied on
ovsMsg->nlMsg anytime, but it is a general goodness check.

>+
>     for (i = 0; i < nlFamilyOps->opsCount; i++) {
>         if (nlFamilyOps->cmds[i].cmd == ovsMsg->genlMsg.cmd) {
>             /* Validate if the command is valid for the device
>operation. */
>@@ -977,6 +1004,14 @@ ValidateNetlinkCmd(UINT32 devOp,
>         }
>     }
> 
>+    // validate all NlAttrs
>+    if (!NlValidateAllAttrs(&ovsMsg->nlMsg, sizeof(*ovsMsg),
>+                            NlMsgAttrsLen((PNL_MSG_HDR)ovsMsg),
>+                            NULL, 0)) {
>+        status = STATUS_INVALID_PARAMETER;
>+        goto done;
>+    }

I am not fully convinced this check will buy us anything. The reason is
that we do the same check later in each of the handler functions along
with the appropriate policy. In fact, the new code parses attributes twice
for no additional benefit. It only contributes to increased CPU usage, but
I see your point that we¹ll have a centralized location to parse all of
the attributes. Maybe useful. Let me think about it more.

>+
> done:
>     return status;
> }
>@@ -1028,6 +1063,7 @@ InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT
>usrParamsCtx,
>             POVS_MESSAGE msgIn = NULL;
>             POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>                 usrParamsCtx->outputBuffer;
>+            UINT32 msgErrorLen = usrParamsCtx->outputLength;
> 
>             if (usrParamsCtx->ovsMsg->genlMsg.cmd ==
>OVS_CTRL_CMD_EVENT_NOTIFY ||
>                 usrParamsCtx->ovsMsg->genlMsg.cmd ==
>OVS_CTRL_CMD_READ_NOTIFY) {
>@@ -1043,8 +1079,7 @@ InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT
>usrParamsCtx,
> 
>             ASSERT(msgIn);
>             ASSERT(msgError);
>-            NlBuildErrorMsg(msgIn, msgError, nlError);
>-            *replyLen = msgError->nlMsg.nlmsgLen;
>+            NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError,
>replyLen);
>         }
> 
>         if (*replyLen != 0) {
>@@ -1416,9 +1451,9 @@ cleanup:
>     if (nlError != NL_ERROR_SUCCESS) {
>         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>             usrParamsCtx->outputBuffer;
>+        UINT32 msgErrorLen = usrParamsCtx->outputLength;
> 
>-        NlBuildErrorMsg(msgIn, msgError, nlError);
>-        *replyLen = msgError->nlMsg.nlmsgLen;
>+        NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen);
>     }
> 
>     return STATUS_SUCCESS;
>diff --git a/datapath-windows/ovsext/Flow.c
>b/datapath-windows/ovsext/Flow.c
>index a49a60c..792b614 100644
>--- a/datapath-windows/ovsext/Flow.c
>+++ b/datapath-windows/ovsext/Flow.c
>@@ -285,20 +285,10 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT
>usrParamsCtx,
>         goto done;
>     }
> 
>-    /* Get all the top level Flow attributes */
>-    if ((NlAttrParse(nlMsgHdr, attrOffset, NlMsgAttrsLen(nlMsgHdr),
>-                     nlFlowPolicy, ARRAY_SIZE(nlFlowPolicy),
>-                     flowAttrs, ARRAY_SIZE(flowAttrs)))
>-                     != TRUE) {
>-        OVS_LOG_ERROR("Attr Parsing failed for msg: %p",
>-                       nlMsgHdr);
>-        rc = STATUS_INVALID_PARAMETER;
>-        goto done;
>-    }
>-
>-    /* FLOW_DEL command w/o any key input is a flush case. */
>+    /* FLOW_DEL command w/o any key input is a flush case.
>+       If we don't have any attr, we treat this as a flush command*/
>     if ((genlMsgHdr->cmd == OVS_FLOW_CMD_DEL) &&
>-        (!(flowAttrs[OVS_FLOW_ATTR_KEY]))) {
>+        (!NlMsgAttrsLen(nlMsgHdr))) {
> 
>         rc = OvsFlushFlowIoctl(ovsHdr->dp_ifindex);
> 
>@@ -323,6 +313,17 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT
>usrParamsCtx,
>        goto done;
>     }
> 
>+    /* Get all the top level Flow attributes */
>+    if ((NlAttrParse(nlMsgHdr, attrOffset, NlMsgAttrsLen(nlMsgHdr),
>+        nlFlowPolicy, ARRAY_SIZE(nlFlowPolicy),
>+        flowAttrs, ARRAY_SIZE(flowAttrs)))
>+        != TRUE) {
>+        OVS_LOG_ERROR("Attr Parsing failed for msg: %p",
>+            nlMsgHdr);
>+        rc = STATUS_INVALID_PARAMETER;
>+        goto done;
>+    }
>+
>     if (flowAttrs[OVS_FLOW_ATTR_PROBE]) {
>         rc = OvsProbeSupportedFeature(msgIn,
>flowAttrs[OVS_FLOW_ATTR_KEY]);
>         if (rc != STATUS_SUCCESS) {
>@@ -399,8 +400,9 @@ done:
>     if (nlError != NL_ERROR_SUCCESS) {
>         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>                                        usrParamsCtx->outputBuffer;
>-        NlBuildErrorMsg(msgIn, msgError, nlError);
>-        *replyLen = msgError->nlMsg.nlmsgLen;
>+        UINT32 msgErrorLen = usrParamsCtx->outputLength;
>+
>+        NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen);
>         rc = STATUS_SUCCESS;
>     }
> 
>@@ -568,8 +570,9 @@ done:
>     if (nlError != NL_ERROR_SUCCESS) {
>         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>                                       usrParamsCtx->outputBuffer;
>-        NlBuildErrorMsg(msgIn, msgError, nlError);
>-        *replyLen = msgError->nlMsg.nlmsgLen;
>+        UINT32 msgErrorLen = usrParamsCtx->outputLength;
>+
>+        NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen);
>         rc = STATUS_SUCCESS;
>     }
> 
>@@ -706,8 +709,9 @@ done:
>     if (nlError != NL_ERROR_SUCCESS) {
>         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>                                       usrParamsCtx->outputBuffer;
>-        NlBuildErrorMsg(msgIn, msgError, nlError);
>-        *replyLen = msgError->nlMsg.nlmsgLen;
>+        UINT32 msgErrorLen = usrParamsCtx->outputLength;
>+
>+        NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen);
>         rc = STATUS_SUCCESS;
>     }
> 
>diff --git a/datapath-windows/ovsext/Netlink/Netlink.c
>b/datapath-windows/ovsext/Netlink/Netlink.c
>index 27dcd4f..dc1e78c 100644
>--- a/datapath-windows/ovsext/Netlink/Netlink.c
>+++ b/datapath-windows/ovsext/Netlink/Netlink.c
>@@ -108,12 +108,18 @@ NlFillNlHdr(PNL_BUFFER nlBuf, UINT16 nlmsgType,
>  * 
>--------------------------------------------------------------------------
>-
>  */
> VOID
>-NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgError, UINT
>errorCode)
>+NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgError,
>+                UINT32 msgErrorLen,
>+                UINT errorCode, UINT32 *msgLen)
> {
>     NL_BUFFER nlBuffer;
> 
>     ASSERT(errorCode != NL_ERROR_PENDING);
> 
>+    if ((msgError == NULL) || (msgErrorLen < sizeof *msgError)) {
>+        return;
>+    }

I am not sure if it is a good idea to silently return here. If ŒmsgError¹
is NULL, it is a contract violation, and must be caught. This is why we
have ASSERTs for msgError before the call to NlBuildErrorMsg. Yes, ASSERTs
are compiled out in !DBG code, but I am ok with taking a BSOD and fixing
an issue rather than silently not report an error.

For checking of msgError, I¹d rather increase the amount of memory we map
in MapIrpOutputBuffer(), and bail out there itself.

>+
>     NlBufInit(&nlBuffer, (PCHAR)msgError, sizeof *msgError);
>     NlFillNlHdr(&nlBuffer, NLMSG_ERROR, 0,
>                 msgIn->nlMsg.nlmsgSeq, msgIn->nlMsg.nlmsgPid);
>@@ -121,6 +127,10 @@ NlBuildErrorMsg(POVS_MESSAGE msgIn,
>POVS_MESSAGE_ERROR msgError, UINT errorCode)
>     msgError->errorMsg.error = errorCode;
>     msgError->errorMsg.nlMsg = msgIn->nlMsg;
>     msgError->nlMsg.nlmsgLen = sizeof(OVS_MESSAGE_ERROR);
>+
>+    if (NULL != msgLen) {
>+        *msgLen = msgError->nlMsg.nlmsgLen;
>+    }

Sure, this is a good cleanup to set replyLen here itself rather than
outside of the function.

> }
> 
> /*
>@@ -1006,7 +1016,7 @@ NlAttrFind__(const PNL_ATTR attrs, UINT32 size,
>UINT16 type)
> {
>     PNL_ATTR iter = NULL;
>     PNL_ATTR ret = NULL;
>-    UINT32 left;
>+    INT left;
> 
>     NL_ATTR_FOR_EACH (iter, left, attrs, size) {
>         if (NlAttrType(iter) == type) {
>@@ -1036,6 +1046,49 @@ NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
> 
> /*
>  
>*-------------------------------------------------------------------------
>---
>+ * Traverses all attributes in received buffer in order to insure all
>are valid
>+ 
>*-------------------------------------------------------------------------
>---
>+ */
>+BOOLEAN NlValidateAllAttrs(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
>+                           UINT32 totalAttrLen,
>+                           const NL_POLICY policy[], const UINT32
>numPolicy)
>+{
>+    PNL_ATTR nla;
>+    INT left;
>+    BOOLEAN ret = TRUE;
>+
>+    if ((NlMsgSize(nlMsg) < attrOffset)) {
>+        OVS_LOG_WARN("No attributes in nlMsg: %p at offset: %d",
>+            nlMsg, attrOffset);
>+        ret = FALSE;
>+        goto done;
>+    }
>+
>+    NL_ATTR_FOR_EACH_UNSAFE(nla, left, NlMsgAt(nlMsg, attrOffset),
>+                            totalAttrLen)
>+    {
>+        if (!NlAttrIsValid(nla, left)) {
>+            ret = FALSE;
>+            goto done;
>+        }
>+
>+        UINT16 type = NlAttrType(nla);
>+        if (type < numPolicy && policy[type].type != NL_A_NO_ATTR) {
>+            /* Typecasting to keep the compiler happy */
>+            const PNL_POLICY e = (const PNL_POLICY)(&policy[type]);
>+            if (!NlAttrValidate(nla, e)) {
>+                ret = FALSE;
>+                goto done;
>+            }
>+        }
>+    }
>+
>+done:
>+    return ret;
>+}
>+
>+/*
>+ 
>*-------------------------------------------------------------------------
>---
>  * Parses the netlink message at a given offset (attrOffset)
>  * as a series of attributes. A pointer to the attribute with type
>  * 'type' is stored in attrs at index 'type'. policy is used to define
>the
>@@ -1052,20 +1105,13 @@ NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32
>attrOffset,
>             PNL_ATTR attrs[], UINT32 numAttrs)
> {
>     PNL_ATTR nla;
>-    UINT32 left;
>+    INT left;
>     UINT32 iter;
>     BOOLEAN ret = FALSE;
>     UINT32 numPolicyAttr = MIN(numPolicy, numAttrs);
> 
>     RtlZeroMemory(attrs, numAttrs * sizeof *attrs);
> 
>-
>-    /* There is nothing to parse */
>-    if (!(NlMsgAttrsLen(nlMsg))) {
>-        ret = TRUE;
>-        goto done;
>-    }
>-
>     if ((NlMsgSize(nlMsg) < attrOffset)) {
>         OVS_LOG_WARN("No attributes in nlMsg: %p at offset: %d",
>                      nlMsg, attrOffset);
>diff --git a/datapath-windows/ovsext/Netlink/Netlink.h
>b/datapath-windows/ovsext/Netlink/Netlink.h
>index 8f6a5be..63164c7 100644
>--- a/datapath-windows/ovsext/Netlink/Netlink.h
>+++ b/datapath-windows/ovsext/Netlink/Netlink.h
>@@ -72,6 +72,7 @@ typedef struct _NL_POLICY
> /* This macro is careful to check for attributes with bad lengths. */
> #define NL_ATTR_FOR_EACH(ITER, LEFT, ATTRS, ATTRS_LEN)                  \
>     for ((ITER) = (ATTRS), (LEFT) = (ATTRS_LEN);                        \
>+         ((INT)LEFT) >= (INT)NLA_ALIGN(sizeof(NL_ATTR)) &&              \
>          NlAttrIsValid(ITER, LEFT);                                     \
>          (LEFT) -= NlAttrLenPad(ITER, LEFT), (ITER) = NlAttrNext(ITER))
> 
>@@ -80,7 +81,7 @@ typedef struct _NL_POLICY
>  * already been validated (e.g. with NL_ATTR_FOR_EACH).  */
> #define NL_ATTR_FOR_EACH_UNSAFE(ITER, LEFT, ATTRS, ATTRS_LEN)           \
>     for ((ITER) = (ATTRS), (LEFT) = (ATTRS_LEN);                        \
>-         (LEFT) > 0;                                                    \
>+         ((INT)LEFT) >= (INT)NLA_ALIGN(sizeof(NL_ATTR));                \
>          (LEFT) -= NLA_ALIGN((ITER)->nlaLen), (ITER) = NlAttrNext(ITER))
> 
> #define NL_ATTR_GET_AS(NLA, TYPE) \
>@@ -94,8 +95,9 @@ BOOLEAN NlFillNlHdr(PNL_BUFFER nlBuf,
>                     UINT16 nlmsgType, UINT16 nlmsgFlags,
>                     UINT32 nlmsgSeq, UINT32 nlmsgPid);
> 
>-VOID NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgOut,
>-                     UINT errorCode);
>+VOID NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgError,
>+                     UINT32 msgErrorLen,
>+                     UINT errorCode, UINT32 *msgLen);
> 
> /* Netlink message accessing the payload */
> PVOID NlMsgAt(const PNL_MSG_HDR nlh, UINT32 offset);
>@@ -187,6 +189,11 @@ static __inline NlAttrIsLast(const PNL_ATTR nla, int
>rem)
> /* Netlink attribute validation */
> BOOLEAN NlAttrValidate(const PNL_ATTR, const PNL_POLICY);
> 
>+/* Netlink attribute stream validation */
>+BOOLEAN NlValidateAllAttrs(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
>+                       UINT32 totalAttrLen,
>+                       const NL_POLICY policy[], const UINT32 numPolicy);
>+
> /* Put APis */
> BOOLEAN NlMsgPutNlHdr(PNL_BUFFER buf, PNL_MSG_HDR nlMsg);
> BOOLEAN NlMsgPutGenlHdr(PNL_BUFFER buf, PGENL_MSG_HDR genlMsg);
>diff --git a/datapath-windows/ovsext/User.c
>b/datapath-windows/ovsext/User.c
>index 33cbd89..bc010c7 100644
>--- a/datapath-windows/ovsext/User.c
>+++ b/datapath-windows/ovsext/User.c
>@@ -345,8 +345,9 @@ OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT
>usrParamsCtx,
> 
>             POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>                                            usrParamsCtx->outputBuffer;
>-            NlBuildErrorMsg(msgIn, msgError, nlError);
>-            *replyLen = msgError->nlMsg.nlmsgLen;
>+            UINT32 msgErrorLen = usrParamsCtx->outputLength;
>+
>+            NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError,
>replyLen);
>             status = STATUS_SUCCESS;
>             goto done;
>         }
>diff --git a/datapath-windows/ovsext/Vport.c
>b/datapath-windows/ovsext/Vport.c
>index d04b12b..4299169 100644
>--- a/datapath-windows/ovsext/Vport.c
>+++ b/datapath-windows/ovsext/Vport.c
>@@ -1729,9 +1729,9 @@ cleanup:
>     if (nlError != NL_ERROR_SUCCESS) {
>         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>             usrParamsCtx->outputBuffer;
>+        UINT32 msgErrorLen = usrParamsCtx->outputLength;
> 
>-        NlBuildErrorMsg(msgIn, msgError, nlError);
>-        *replyLen = msgError->nlMsg.nlmsgLen;
>+        NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen);
>     }
> 
>     return STATUS_SUCCESS;
>@@ -2088,9 +2088,9 @@ Cleanup:
>     if (nlError != NL_ERROR_SUCCESS) {
>         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>             usrParamsCtx->outputBuffer;
>+        UINT32 msgErrorLen = usrParamsCtx->outputLength;
> 
>-        NlBuildErrorMsg(msgIn, msgError, nlError);
>-        *replyLen = msgError->nlMsg.nlmsgLen;
>+        NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen);
>     }
> 
>     return STATUS_SUCCESS;
>@@ -2324,6 +2324,7 @@ Cleanup:
>     if ((nlError != NL_ERROR_SUCCESS) && (nlError != NL_ERROR_PENDING)) {
>         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>             usrParamsCtx->outputBuffer;
>+        UINT32 msgErrorLen = usrParamsCtx->outputLength;
> 
>         if (vport && vportAllocated == TRUE) {
>             if (vportInitialized == TRUE) {
>@@ -2343,8 +2344,7 @@ Cleanup:
>             OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG);
>         }
> 
>-        NlBuildErrorMsg(msgIn, msgError, nlError);
>-        *replyLen = msgError->nlMsg.nlmsgLen;
>+        NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen);
>     }
> 
>     return (status == STATUS_PENDING) ? STATUS_PENDING : STATUS_SUCCESS;
>@@ -2452,9 +2452,9 @@ Cleanup:
>     if (nlError != NL_ERROR_SUCCESS) {
>         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>             usrParamsCtx->outputBuffer;
>+        UINT32 msgErrorLen = usrParamsCtx->outputLength;
> 
>-        NlBuildErrorMsg(msgIn, msgError, nlError);
>-        *replyLen = msgError->nlMsg.nlmsgLen;
>+        NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen);
>     }
> 
>     return STATUS_SUCCESS;
>@@ -2544,9 +2544,9 @@ Cleanup:
>     if ((nlError != NL_ERROR_SUCCESS) && (nlError != NL_ERROR_PENDING)) {
>         POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>             usrParamsCtx->outputBuffer;
>+        UINT32 msgErrorLen = usrParamsCtx->outputLength;
> 
>-        NlBuildErrorMsg(msgIn, msgError, nlError);
>-        *replyLen = msgError->nlMsg.nlmsgLen;
>+        NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen);
>     }
> 
>     return (status == STATUS_PENDING) ? STATUS_PENDING : STATUS_SUCCESS;
>@@ -2579,10 +2579,11 @@ OvsTunnelVportPendingRemove(PVOID context,
> 
>             *replyLen = msgOut->nlMsg.nlmsgLen;
>         } else {
>-            POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)msgOut;
>+            POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>+                        tunnelContext->outputBuffer;
>+            UINT32 msgErrorLen = tunnelContext->outputLength;
> 
>-            NlBuildErrorMsg(msgIn, msgError, nlError);
>-            *replyLen = msgError->nlMsg.nlmsgLen;
>+            NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError,
>replyLen);
>         }
>     }
> 
>@@ -2721,12 +2722,13 @@ OvsTunnelVportPendingInit(PVOID context,
>     } while (error);
> 
>     if (error) {
>-        POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) msgOut;
>+        POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
>+            tunnelContext->outputBuffer;
>+        UINT32 msgErrorLen = tunnelContext->outputLength;
> 
>         OvsCleanupVxlanTunnel(NULL, vport, NULL, NULL);
>         OvsFreeMemory(vport);
> 
>-        NlBuildErrorMsg(msgIn, msgError, nlError);
>-        *replyLen = msgError->nlMsg.nlmsgLen;
>+        NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen);
>     }
> }
>diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
>index cad9490..32b0cc3 100644
>--- a/lib/netlink-socket.c
>+++ b/lib/netlink-socket.c
>@@ -127,6 +127,7 @@ nl_sock_create(int protocol, struct nl_sock **sockp)
>     sock = xmalloc(sizeof *sock);
> 
> #ifdef _WIN32
>+    sock->overlapped.hEvent = NULL;

Good catch.

>     sock->handle = CreateFile(OVS_DEVICE_NAME_USER,
>                               GENERIC_READ | GENERIC_WRITE,
>                               FILE_SHARE_READ | FILE_SHARE_WRITE,
>@@ -1191,6 +1192,7 @@ pend_io_request(struct nl_sock *sock)
> 
>     ovs_header = ofpbuf_put_uninit(&request, sizeof *ovs_header);
>     ovs_header->dp_ifindex = 0;
>+    nlmsg->nlmsg_len = request.size;

Good catch.

> 
>     if (!DeviceIoControl(sock->handle, OVS_IOCTL_WRITE,
>                          request.data, request.size,




More information about the dev mailing list