| Message ID | 1461582908-2468-1-git-send-email-pboca@cloudbasesolutions.com |
|---|---|
| State | Superseded |
| Headers | show |
Beside the alignment problems, looks good to me. Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> Thanks, Alin. > -----Mesaj original----- > De la: dev [mailto:dev-bounces@openvswitch.org] În numele Paul Boca > Trimis: Monday, April 25, 2016 2:16 PM > Către: dev@openvswitch.org > Subiect: [ovs-dev] [PATCH] 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@cloudbasesolutions.com> > --- > datapath-windows/ovsext/Datapath.c | 45 +++++++++++++++++--- > datapath-windows/ovsext/Flow.c | 42 ++++++++++--------- > datapath-windows/ovsext/Netlink/Netlink.c | 68 > ++++++++++++++++++++++++++----- datapath- > windows/ovsext/Netlink/Netlink.h | 15 +++++-- > datapath-windows/ovsext/User.c | 5 ++- > datapath-windows/ovsext/Vport.c | 34 ++++++++-------- > lib/netlink-socket.c | 2 + > 7 files changed, 154 insertions(+), 57 deletions(-) > > diff --git a/datapath-windows/ovsext/Datapath.c b/datapath- > windows/ovsext/Datapath.c > index 0a25af0..6f97693 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, @@ -693,6 +694,7 @@ > OvsDeviceControl(PDEVICE_OBJECT deviceObject, > UINT32 devOp; > OVS_MESSAGE ovsMsgReadOp; > POVS_MESSAGE ovsMsg; > + UINT32 ovsMsgLength = 0; > NETLINK_FAMILY *nlFamilyOps; > OVS_USER_PARAMS_CONTEXT usrParamsCtx; > > @@ -772,6 +774,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, > } > > ovsMsg = inputBuffer; > + ovsMsgLength = inputBufferLen; > devOp = OVS_TRANSACTION_DEV_OP; > break; > > @@ -806,6 +809,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; > @@ -851,6 +855,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, > > /* Create an NL message for consumption. */ > ovsMsg = &ovsMsgReadOp; > + ovsMsgLength = sizeof (ovsMsgReadOp); > devOp = OVS_READ_DEV_OP; > > break; > @@ -862,7 +867,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; > > @@ -901,7 +920,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; > } > @@ -936,11 +956,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; > + } > + > 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. */ @@ - > 975,6 +1002,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; > + } > + > done: > return status; > } > @@ -1026,6 +1061,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) { @@ -1041,8 +1077,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) { > @@ -1414,9 +1449,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 a7e9bd2..2a3bf73 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 e2312da..b5a97b9 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; > + } > + > 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; > + } > } > > /* > @@ -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); @@ -1100,7 +1146,7 @@ NlAttrParse(const > PNL_MSG_HDR nlMsg, UINT32 attrOffset, > for (iter = 0; iter < numPolicyAttr; iter++) { > const PNL_POLICY e = (const PNL_POLICY)(&policy[iter]); > if (!e->optional && e->type != NL_A_NO_ATTR && !attrs[iter]) { > - OVS_LOG_ERROR("Required attr:%d missing", iter); > + OVS_LOG_WARN("Required attr:%d missing", iter); > goto done; > } > } > diff --git a/datapath-windows/ovsext/Netlink/Netlink.h b/datapath- > windows/ovsext/Netlink/Netlink.h > index 8f6a5be..a37ea57 100644 > --- a/datapath-windows/ovsext/Netlink/Netlink.h > +++ b/datapath-windows/ovsext/Netlink/Netlink.h > @@ -72,7 +72,8 @@ 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); \ > - NlAttrIsValid(ITER, LEFT); \ > + ((INT)LEFT) >= (INT)NLA_ALIGN(sizeof(NL_ATTR)) && \ > + NlAttrIsValid(ITER, LEFT); \ > (LEFT) -= NlAttrLenPad(ITER, LEFT), (ITER) = NlAttrNext(ITER)) > > /* This macro does not check for attributes with bad lengths. It should only > @@ -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 6b2d94a..909e945 100644 > --- a/datapath-windows/ovsext/User.c > +++ b/datapath-windows/ovsext/User.c > @@ -355,8 +355,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 882b41f..2825a22 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; > 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; > > if (!DeviceIoControl(sock->handle, OVS_IOCTL_WRITE, > request.data, request.size, > -- > 2.7.2.windows.1 > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index 0a25af0..6f97693 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, @@ -693,6 +694,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, UINT32 devOp; OVS_MESSAGE ovsMsgReadOp; POVS_MESSAGE ovsMsg; + UINT32 ovsMsgLength = 0; NETLINK_FAMILY *nlFamilyOps; OVS_USER_PARAMS_CONTEXT usrParamsCtx; @@ -772,6 +774,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, } ovsMsg = inputBuffer; + ovsMsgLength = inputBufferLen; devOp = OVS_TRANSACTION_DEV_OP; break; @@ -806,6 +809,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; @@ -851,6 +855,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, /* Create an NL message for consumption. */ ovsMsg = &ovsMsgReadOp; + ovsMsgLength = sizeof (ovsMsgReadOp); devOp = OVS_READ_DEV_OP; break; @@ -862,7 +867,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; @@ -901,7 +920,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; } @@ -936,11 +956,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; + } + 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. */ @@ -975,6 +1002,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; + } + done: return status; } @@ -1026,6 +1061,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) { @@ -1041,8 +1077,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) { @@ -1414,9 +1449,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 a7e9bd2..2a3bf73 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 e2312da..b5a97b9 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; + } + 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; + } } /* @@ -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); @@ -1100,7 +1146,7 @@ NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset, for (iter = 0; iter < numPolicyAttr; iter++) { const PNL_POLICY e = (const PNL_POLICY)(&policy[iter]); if (!e->optional && e->type != NL_A_NO_ATTR && !attrs[iter]) { - OVS_LOG_ERROR("Required attr:%d missing", iter); + OVS_LOG_WARN("Required attr:%d missing", iter); goto done; } } diff --git a/datapath-windows/ovsext/Netlink/Netlink.h b/datapath-windows/ovsext/Netlink/Netlink.h index 8f6a5be..a37ea57 100644 --- a/datapath-windows/ovsext/Netlink/Netlink.h +++ b/datapath-windows/ovsext/Netlink/Netlink.h @@ -72,7 +72,8 @@ 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); \ - NlAttrIsValid(ITER, LEFT); \ + ((INT)LEFT) >= (INT)NLA_ALIGN(sizeof(NL_ATTR)) && \ + NlAttrIsValid(ITER, LEFT); \ (LEFT) -= NlAttrLenPad(ITER, LEFT), (ITER) = NlAttrNext(ITER)) /* This macro does not check for attributes with bad lengths. It should only @@ -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 6b2d94a..909e945 100644 --- a/datapath-windows/ovsext/User.c +++ b/datapath-windows/ovsext/User.c @@ -355,8 +355,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 882b41f..2825a22 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; 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; if (!DeviceIoControl(sock->handle, OVS_IOCTL_WRITE, request.data, request.size,
Solved access violation when trying to acces netling message - obtained with forged IOCTLs Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> --- datapath-windows/ovsext/Datapath.c | 45 +++++++++++++++++--- datapath-windows/ovsext/Flow.c | 42 ++++++++++--------- datapath-windows/ovsext/Netlink/Netlink.c | 68 ++++++++++++++++++++++++++----- datapath-windows/ovsext/Netlink/Netlink.h | 15 +++++-- datapath-windows/ovsext/User.c | 5 ++- datapath-windows/ovsext/Vport.c | 34 ++++++++-------- lib/netlink-socket.c | 2 + 7 files changed, 154 insertions(+), 57 deletions(-)