| Message ID | 1463697109-3376-2-git-send-email-nithin@vmware.com |
|---|---|
| State | Accepted |
| Headers | show |
Acked-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > -----Original Message----- > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Nithin Raju > Sent: Friday, May 20, 2016 1:32 AM > To: dev@openvswitch.org > Subject: [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 <nithin@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
On 19 May 2016 at 23:56, Paul Boca <pboca@cloudbasesolutions.com> wrote: > Acked-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > >> -----Original Message----- >> From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Nithin Raju >> Sent: Friday, May 20, 2016 1:32 AM >> To: dev@openvswitch.org >> Subject: [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 <nithin@vmware.com> Thanks, applied.
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> > -----Mesaj original----- > De la: dev [mailto:dev-bounces@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 <nithin@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
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); } }
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 <nithin@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(-)