[ovs-dev] [PATCH 2/2 v3] datpaath-windows: consolidate check for output buffer

Nithin Raju nithin at vmware.com
Thu Nov 20 17:06:34 UTC 2014


Earlier, output buffer was optional in a transaction command. Thus each
command handler, had to check if the output buffer had indeed been
specified by userspace or not.

Now that output buffer is mandatory in transaction command, let's
consolidate the check in one place, and also convert the previous check
to ASSERTs.

Signed-off-by: Nithin Raju <nithin at vmware.com>
Acked-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
---
 datapath-windows/ovsext/Datapath.c |   38 ++++++++++++++++++-----------------
 1 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index b67ba44..cd7da07 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -705,7 +705,7 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
      */
     switch (code) {
     case OVS_IOCTL_TRANSACT:
-        /* Input buffer is mandatory, output buffer is optional. */
+        /* Both input buffer and output buffer are mandatory. */
         if (outputBufferLen != 0) {
             status = MapIrpOutputBuffer(irp, outputBufferLen,
                                         sizeof *ovsMsg, &outputBuffer);
@@ -713,6 +713,9 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
                 goto done;
             }
             ASSERT(outputBuffer);
+        } else {
+            status = STATUS_NDIS_INVALID_LENGTH;
+            goto done;
         }
 
         if (inputBufferLen < sizeof (*ovsMsg)) {
@@ -726,7 +729,10 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
 
     case OVS_IOCTL_READ_EVENT:
     case OVS_IOCTL_READ_PACKET:
-        /* This IOCTL is used to read events */
+        /*
+         * Output buffer is mandatory. These IOCTLs are used to read events and
+         * packets respectively. It is convenient to have separate ioctls.
+         */
         if (outputBufferLen != 0) {
             status = MapIrpOutputBuffer(irp, outputBufferLen,
                                         sizeof *ovsMsg, &outputBuffer);
@@ -1180,6 +1186,7 @@ HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 
         /* Dump state must have been deleted after previous dump operation. */
         ASSERT(instance->dumpState.index[0] == 0);
+
         /* Output buffer has been validated while validating read dev op. */
         ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof *msgOut);
 
@@ -1268,10 +1275,9 @@ HandleDpTransactionCommon(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         RtlZeroMemory(dpAttrs, sizeof dpAttrs);
     }
 
-    /* Output buffer is optional for OVS_TRANSACTION_DEV_OP. */
-    if (msgOut == NULL || usrParamsCtx->outputLength < sizeof *msgOut) {
-        return STATUS_NDIS_INVALID_LENGTH;
-    }
+    /* Output buffer has been validated while validating transact dev op. */
+    ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof *msgOut);
+
     NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength);
 
     OvsAcquireCtrlLock();
@@ -1613,9 +1619,8 @@ OvsGetVport(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         return STATUS_INVALID_PARAMETER;
     }
 
-    if (msgOut == NULL || usrParamsCtx->outputLength < sizeof *msgOut) {
-        return STATUS_INVALID_BUFFER_SIZE;
-    }
+    /* Output buffer has been validated while validating transact dev op. */
+    ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof *msgOut);
 
     NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, 0);
     if (vportAttrs[OVS_VPORT_ATTR_NAME] != NULL) {
@@ -1746,9 +1751,8 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     /* input buffer has been validated while validating write dev op. */
     ASSERT(usrParamsCtx->inputBuffer != NULL);
 
-    if (msgOut == NULL || usrParamsCtx->outputLength < sizeof *msgOut) {
-        return STATUS_INVALID_BUFFER_SIZE;
-    }
+    /* Output buffer has been validated while validating transact dev op. */
+    ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof *msgOut);
 
     if (!NlAttrParse((PNL_MSG_HDR)msgIn,
         NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN,
@@ -1938,9 +1942,8 @@ OvsSetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         return STATUS_INVALID_PARAMETER;
     }
 
-    if (msgOut == NULL || usrParamsCtx->outputLength < sizeof *msgOut) {
-        return STATUS_NDIS_INVALID_LENGTH;
-    }
+    /* Output buffer has been validated while validating transact dev op. */
+    ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof *msgOut);
 
     OvsAcquireCtrlLock();
 
@@ -2044,9 +2047,8 @@ OvsDeleteVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         return STATUS_INVALID_PARAMETER;
     }
 
-    if (msgOut == NULL || usrParamsCtx->outputLength < sizeof *msgOut) {
-        return STATUS_NDIS_INVALID_LENGTH;
-    }
+    /* Output buffer has been validated while validating transact dev op. */
+    ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof *msgOut);
 
     NdisAcquireRWLockWrite(gOvsSwitchContext->dispatchLock, &lockState, 0);
     if (vportAttrs[OVS_VPORT_ATTR_NAME] != NULL) {
-- 
1.7.4.1




More information about the dev mailing list