From 32654528b82a0faebeb250c2680ce2088f6aa74d Mon Sep 17 00:00:00 2001 From: Sairam Venugopal Date: Mon, 25 Jul 2016 17:04:38 -0700 Subject: datapath-windows: Fix bugs in Event.c around subscribe and lock 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. Signed-off-by: Sairam Venugopal Acked-By: Yin Lin Acked-by: Alin Gabriel Serdean Signed-off-by: Gurucharan Shetty --- datapath-windows/ovsext/Datapath.c | 7 ++++++- datapath-windows/ovsext/Event.c | 6 ++---- 2 files changed, 8 insertions(+), 5 deletions(-) (limited to 'datapath-windows/ovsext') diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index e4d6ab18d..75f133a90 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 8c7c3ec93..8ff0322fa 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"); -- cgit v1.2.1