When userspace tries to resubscribe to an existing queue, return
STATUS_INVALID_PARAMETER since it's not supported. The current bug
overwrites status to STATUS_SUCCESS.

The second bug fix is around releasing the EventQueue lock if an open
instance couldn't be found. The current version returns back without
releasing the lock. Moving the OvsAcquireEventQueueLock() after the
instance is verified.

OvsGetOpenInstance does not enforce a safe read for
gOvsSwitchContext->dpNo. Use the gOvsSwitchContext->dispatchLock for
accessing the parameter.

v2: Address review comments from Alin Serdean and Yin Lin (around keeping
OvsGetOpenInstance safe).

Signed-off-by: Sairam Venugopal <vsai...@vmware.com>
---
 datapath-windows/ovsext/Datapath.c | 7 ++++++-
 datapath-windows/ovsext/Event.c    | 6 ++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index e4d6ab1..75f133a 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -521,12 +521,17 @@ POVS_OPEN_INSTANCE
 OvsGetOpenInstance(PFILE_OBJECT fileObject,
                    UINT32 dpNo)
 {
+    LOCK_STATE_EX lockState;
     POVS_OPEN_INSTANCE instance = (POVS_OPEN_INSTANCE)fileObject->FsContext;
     ASSERT(instance);
     ASSERT(instance->fileObject == fileObject);
+    NdisAcquireRWLockWrite(gOvsSwitchContext->dispatchLock, &lockState, 0);
+
     if (gOvsSwitchContext->dpNo != dpNo) {
-        return NULL;
+        instance = NULL;
     }
+
+    NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState);
     return instance;
 }
 
diff --git a/datapath-windows/ovsext/Event.c b/datapath-windows/ovsext/Event.c
index 8c7c3ec..8ff0322 100644
--- a/datapath-windows/ovsext/Event.c
+++ b/datapath-windows/ovsext/Event.c
@@ -217,9 +217,8 @@ OvsSubscribeEventIoctl(PFILE_OBJECT fileObject,
         if (queue->mask != request->mask) {
             status = STATUS_INVALID_PARAMETER;
             OVS_LOG_WARN("Can not chnage mask when the queue is subscribed");
+            goto done_event_subscribe;
         }
-        status = STATUS_SUCCESS;
-        goto done_event_subscribe;
     } else if (!request->subscribe && queue == NULL) {
         status = STATUS_SUCCESS;
         goto done_event_subscribe;
@@ -356,8 +355,6 @@ OvsWaitEventIoctl(PIRP irp,
     }
     poll = (POVS_EVENT_POLL)inputBuffer;
 
-    OvsAcquireEventQueueLock();
-
     instance = OvsGetOpenInstance(fileObject, poll->dpNo);
     if (instance == NULL) {
         OVS_LOG_TRACE("Exit: Can not find open instance, dpNo: %d",
@@ -365,6 +362,7 @@ OvsWaitEventIoctl(PIRP irp,
         return STATUS_INVALID_PARAMETER;
     }
 
+    OvsAcquireEventQueueLock();
     queue = (POVS_EVENT_QUEUE)instance->eventQueue;
     if (queue == NULL) {
         OVS_LOG_TRACE("Exit: Event queue does not exist");
-- 
2.9.0.windows.1

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to