Acked-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com>
> -----Mesaj original----- > De la: dev [mailto:dev-boun...@openvswitch.org] În numele Nithin Raju > Trimis: Friday, May 20, 2016 1:32 AM > Către: dev@openvswitch.org > Subiect: [ovs-dev] [PATCH 2/2] datapath-windows: o/p buffer must fit NL > error message > > OVS_IOCTL_WRITE and OVS_IOCTL_TRANSACT can generate a netlink error > that is represented by a OVS_MESSAGE_ERROR struct. We want to make > sure at the entry point of the ioctl processing that the output buffer is big > enough to hold the error message. We were earlier checking for struct > OVS_MESSAGE which is smaller. > > Since we are ensuring that output buffer can fit OVS_MESSAGE_ERROR at > the top of the ioctl function, there's no need to check for that later. > > Signed-off-by: Nithin Raju <nit...@vmware.com> > --- > datapath-windows/ovsext/Datapath.c | 19 ++++++++------ > datapath-windows/ovsext/Flow.c | 16 ++++++------ > datapath-windows/ovsext/Netlink/Netlink.c | 11 ++------ datapath- > windows/ovsext/Netlink/Netlink.h | 1 - > datapath-windows/ovsext/User.c | 4 +-- > datapath-windows/ovsext/Vport.c | 42 +++++++++++++++++----------- > --- > 6 files changed, 48 insertions(+), 45 deletions(-) > > diff --git a/datapath-windows/ovsext/Datapath.c b/datapath- > windows/ovsext/Datapath.c > index e33027c..b2c7020 100644 > --- a/datapath-windows/ovsext/Datapath.c > +++ b/datapath-windows/ovsext/Datapath.c > @@ -759,8 +759,10 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, > case OVS_IOCTL_TRANSACT: > /* Both input buffer and output buffer are mandatory. */ > if (outputBufferLen != 0) { > + ASSERT(sizeof(OVS_MESSAGE_ERROR) >= sizeof *ovsMsg); > status = MapIrpOutputBuffer(irp, outputBufferLen, > - sizeof *ovsMsg, &outputBuffer); > + sizeof(OVS_MESSAGE_ERROR), > + &outputBuffer); > if (status != STATUS_SUCCESS) { > goto done; > } > @@ -788,7 +790,8 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, > */ > if (outputBufferLen != 0) { > status = MapIrpOutputBuffer(irp, outputBufferLen, > - sizeof *ovsMsg, &outputBuffer); > + sizeof(OVS_MESSAGE_ERROR), > + &outputBuffer); > if (status != STATUS_SUCCESS) { > goto done; > } > @@ -820,7 +823,8 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject, > /* Output buffer is mandatory. */ > if (outputBufferLen != 0) { > status = MapIrpOutputBuffer(irp, outputBufferLen, > - sizeof *ovsMsg, &outputBuffer); > + sizeof(OVS_MESSAGE_ERROR), > + &outputBuffer); > if (status != STATUS_SUCCESS) { > goto done; > } > @@ -1050,7 +1054,6 @@ > 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) { @@ -1066,7 +1069,8 @@ > InvokeNetlinkCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, > > ASSERT(msgIn); > ASSERT(msgError); > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > } > > if (*replyLen != 0) { > @@ -1438,9 +1442,10 @@ cleanup: > if (nlError != NL_ERROR_SUCCESS) { > POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) > usrParamsCtx->outputBuffer; > - UINT32 msgErrorLen = usrParamsCtx->outputLength; > > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > } > > return STATUS_SUCCESS; > diff --git a/datapath-windows/ovsext/Flow.c b/datapath- > windows/ovsext/Flow.c index d9b4f2a..c2e0227 100644 > --- a/datapath-windows/ovsext/Flow.c > +++ b/datapath-windows/ovsext/Flow.c > @@ -398,9 +398,10 @@ done: > if (nlError != NL_ERROR_SUCCESS) { > POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) > usrParamsCtx->outputBuffer; > - UINT32 msgErrorLen = usrParamsCtx->outputLength; > > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > rc = STATUS_SUCCESS; > } > > @@ -568,9 +569,10 @@ done: > if (nlError != NL_ERROR_SUCCESS) { > POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) > usrParamsCtx->outputBuffer; > - UINT32 msgErrorLen = usrParamsCtx->outputLength; > > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > rc = STATUS_SUCCESS; > } > > @@ -707,9 +709,9 @@ done: > if (nlError != NL_ERROR_SUCCESS) { > POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) > usrParamsCtx->outputBuffer; > - UINT32 msgErrorLen = usrParamsCtx->outputLength; > - > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > rc = STATUS_SUCCESS; > } > > diff --git a/datapath-windows/ovsext/Netlink/Netlink.c b/datapath- > windows/ovsext/Netlink/Netlink.c > index dc1e78c..1eec320 100644 > --- a/datapath-windows/ovsext/Netlink/Netlink.c > +++ b/datapath-windows/ovsext/Netlink/Netlink.c > @@ -109,17 +109,12 @@ NlFillNlHdr(PNL_BUFFER nlBuf, UINT16 nlmsgType, > */ > VOID > NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgError, > - UINT32 msgErrorLen, > - UINT errorCode, UINT32 *msgLen) > + UINT errorCode, UINT32 *replyLen) > { > 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); @@ -128,9 > +123,7 @@ NlBuildErrorMsg(POVS_MESSAGE msgIn, > POVS_MESSAGE_ERROR msgError, > msgError->errorMsg.nlMsg = msgIn->nlMsg; > msgError->nlMsg.nlmsgLen = sizeof(OVS_MESSAGE_ERROR); > > - if (NULL != msgLen) { > - *msgLen = msgError->nlMsg.nlmsgLen; > - } > + *replyLen = msgError->nlMsg.nlmsgLen; > } > > /* > diff --git a/datapath-windows/ovsext/Netlink/Netlink.h b/datapath- > windows/ovsext/Netlink/Netlink.h > index 63164c7..b1b3bed 100644 > --- a/datapath-windows/ovsext/Netlink/Netlink.h > +++ b/datapath-windows/ovsext/Netlink/Netlink.h > @@ -96,7 +96,6 @@ BOOLEAN NlFillNlHdr(PNL_BUFFER nlBuf, > UINT32 nlmsgSeq, UINT32 nlmsgPid); > > VOID NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR > msgError, > - UINT32 msgErrorLen, > UINT errorCode, UINT32 *msgLen); > > /* Netlink message accessing the payload */ diff --git a/datapath- > windows/ovsext/User.c b/datapath-windows/ovsext/User.c index > bb90a83..92a71e1 100644 > --- a/datapath-windows/ovsext/User.c > +++ b/datapath-windows/ovsext/User.c > @@ -348,9 +348,9 @@ > OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, > > POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) > usrParamsCtx->outputBuffer; > - UINT32 msgErrorLen = usrParamsCtx->outputLength; > > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > status = STATUS_SUCCESS; > goto done; > } > diff --git a/datapath-windows/ovsext/Vport.c b/datapath- > windows/ovsext/Vport.c index 4299169..222b2c1 100644 > --- a/datapath-windows/ovsext/Vport.c > +++ b/datapath-windows/ovsext/Vport.c > @@ -1729,9 +1729,10 @@ cleanup: > if (nlError != NL_ERROR_SUCCESS) { > POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) > usrParamsCtx->outputBuffer; > - UINT32 msgErrorLen = usrParamsCtx->outputLength; > > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > } > > return STATUS_SUCCESS; > @@ -2088,9 +2089,10 @@ Cleanup: > if (nlError != NL_ERROR_SUCCESS) { > POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) > usrParamsCtx->outputBuffer; > - UINT32 msgErrorLen = usrParamsCtx->outputLength; > > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > } > > return STATUS_SUCCESS; > @@ -2324,7 +2326,6 @@ 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) { @@ -2344,7 +2345,9 @@ Cleanup: > OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG); > } > > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > } > > return (status == STATUS_PENDING) ? STATUS_PENDING : > STATUS_SUCCESS; @@ -2452,9 +2455,10 @@ Cleanup: > if (nlError != NL_ERROR_SUCCESS) { > POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) > usrParamsCtx->outputBuffer; > - UINT32 msgErrorLen = usrParamsCtx->outputLength; > > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > } > > return STATUS_SUCCESS; > @@ -2544,9 +2548,10 @@ 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, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > } > > return (status == STATUS_PENDING) ? STATUS_PENDING : > STATUS_SUCCESS; @@ -2579,11 +2584,10 @@ > OvsTunnelVportPendingRemove(PVOID context, > > *replyLen = msgOut->nlMsg.nlmsgLen; > } else { > - POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) > - tunnelContext->outputBuffer; > - UINT32 msgErrorLen = tunnelContext->outputLength; > - > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + POVS_MESSAGE_ERROR msgError = > (POVS_MESSAGE_ERROR)msgOut; > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > } > } > > @@ -2722,13 +2726,13 @@ OvsTunnelVportPendingInit(PVOID context, > } while (error); > > if (error) { > - POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) > - tunnelContext->outputBuffer; > - UINT32 msgErrorLen = tunnelContext->outputLength; > + POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)msgOut; > > OvsCleanupVxlanTunnel(NULL, vport, NULL, NULL); > OvsFreeMemory(vport); > > - NlBuildErrorMsg(msgIn, msgError, msgErrorLen, nlError, replyLen); > + ASSERT(msgError); > + NlBuildErrorMsg(msgIn, msgError, nlError, replyLen); > + ASSERT(*replyLen != 0); > } > } > -- > 2.7.1.windows.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
