Hi Nithin! Thanks for your review and changes!
Regards, Paul > -----Original Message----- > From: Nithin Raju [mailto:nit...@vmware.com] > Sent: Friday, May 20, 2016 1:31 AM > To: Paul Boca; dev@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-boun...@openvswitch.org> on behalf of Paul Boca > <pb...@cloudbasesolutions.com> > Date: Wednesday, April 27, 2016 at 1:05 AM > To: "dev@openvswitch.org" <dev@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 <pb...@cloudbasesolutions.com> > >Acked-by: Alin Gabriel Serdean <aserd...@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, _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
