[ovs-dev] [PATCH V2] datapath-windows: Validate netlink packets integrity
Paul Boca
pboca at cloudbasesolutions.com
Fri May 20 07:10:52 UTC 2016
Hi Nithin!
Thanks for your review and changes!
Regards,
Paul
> -----Original Message-----
> From: Nithin Raju [mailto:nithin at vmware.com]
> Sent: Friday, May 20, 2016 1:31 AM
> To: Paul Boca; dev at openvswitch.org
> Subject: Re: [ovs-dev] [PATCH V2] datapath-windows: Validate netlink packets
> integrity
>
> 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