[ovs-dev] [PATCH 2/2] datapath-windows: o/p buffer must fit NL error message
Alin Serdean
aserdean at cloudbasesolutions.com
Wed May 25 17:41:28 UTC 2016
Acked-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
> -----Mesaj original-----
> De la: dev [mailto:dev-bounces at openvswitch.org] În numele Nithin Raju
> Trimis: Friday, May 20, 2016 1:32 AM
> Către: dev at 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 at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list