Fixes hang in DeviceIoControl

Contributors.asciidoc

@@ -56,4 +56,5 @@ CONTRIBUTOR LIST
 |===
 |Bill Zissimopoulos                     |billziss at navimatics.com
 |VRan Liu                               |gliuwr at gmail.com
+|Aleks Margarian                        |aleks.margarian1 at gmail.com
 |===

inc/winspd/ioctl.h

@@ -215,7 +215,8 @@ DWORD SpdIoctlTransact(HANDLE DeviceHandle,
     UINT32 Btl,
     SPD_IOCTL_TRANSACT_RSP *Rsp,
     SPD_IOCTL_TRANSACT_REQ *Req,
-    PVOID DataBuffer);
+    PVOID DataBuffer,
+    OVERLAPPED *Overlapped);
 DWORD SpdIoctlSetTransactProcessId(HANDLE DeviceHandle,
     UINT32 Btl,
     ULONG ProcessId);

src/shared/ioctl.c

@@ -412,7 +412,8 @@ DWORD SpdIoctlTransact(HANDLE DeviceHandle,
     UINT32 Btl,
     SPD_IOCTL_TRANSACT_RSP *Rsp,
     SPD_IOCTL_TRANSACT_REQ *Req,
-    PVOID DataBuffer)
+    PVOID DataBuffer,
+    OVERLAPPED *Overlapped)
 {
     SPD_IOCTL_TRANSACT_PARAMS Params;
     DWORD BytesTransferred;
@@ -452,13 +453,26 @@ DWORD SpdIoctlTransact(HANDLE DeviceHandle,
      * FILE_FLAG_OVERLAPPED flag we ensure that the unproductive serialization
      * does not happen.
      */
+     // DeviceIoControl() can hang if lpOverlapped is NULL
     if (!DeviceIoControl(DeviceHandle, IOCTL_MINIPORT_PROCESS_SERVICE_IRP,
         &Params, sizeof Params,
         &Params, sizeof Params,
-        &BytesTransferred, 0))
+        &BytesTransferred, Overlapped))
     {
         Error = GetLastError();
-        goto exit;
+        if (ERROR_IO_PENDING == Error)
+        {
+            if (!GetOverlappedResult(DeviceHandle, Overlapped,
+                &BytesTransferred, TRUE))
+            {
+                Error = GetLastError();
+                goto exit;
+            }
+        }
+        else
+        {
+            goto exit;
+        }
     }
 
     if (0 != Req)

src/shared/stghandle.c

@@ -278,24 +278,20 @@ static DWORD SpdStorageUnitHandleTransactPipe(HANDLE Handle,
     UINT32 Btl,
     SPD_IOCTL_TRANSACT_RSP *Rsp,
     SPD_IOCTL_TRANSACT_REQ *Req,
-    PVOID DataBuffer)
+    PVOID DataBuffer,
+    OVERLAPPED *Overlapped)
 {
     STORAGE_UNIT *StorageUnit = Handle;
     LONG Connected;
     ULONG DataLength;
     TRANSACT_MSG *Msg = 0;
-    OVERLAPPED Overlapped;
     DWORD BytesTransferred;
     DWORD Error;
 
     if ((0 == Req && 0 == Rsp) ||
         (0 != Req && 0 == DataBuffer))
         return ERROR_INVALID_PARAMETER;
 
-    Error = SpdOverlappedInit(&Overlapped);
-    if (ERROR_SUCCESS != Error)
-        goto exit;
-
     AcquireSRWLockShared(&StorageUnitLock);
     Error = StorageUnit == StorageUnits[SPD_INDEX_FROM_BTL(Btl)] ?
         ERROR_SUCCESS : ERROR_FILE_NOT_FOUND;
@@ -316,16 +312,16 @@ static DWORD SpdStorageUnitHandleTransactPipe(HANDLE Handle,
     {
         Error = WaitOverlappedResult(
             StorageUnit->Event,
-            ConnectNamedPipe(StorageUnit->Pipe, &Overlapped),
-            StorageUnit->Pipe, &Overlapped, &BytesTransferred);
+            ConnectNamedPipe(StorageUnit->Pipe, Overlapped),
+            StorageUnit->Pipe, Overlapped, &BytesTransferred);
         if (ERROR_SUCCESS == Error || ERROR_PIPE_CONNECTED == Error)
         {
             Error = WaitOverlappedResult(
                 StorageUnit->Event,
                 WriteFile(StorageUnit->Pipe,
                     &StorageUnit->StorageUnitParams, sizeof StorageUnit->StorageUnitParams,
-                    0, &Overlapped),
-                StorageUnit->Pipe, &Overlapped, &BytesTransferred);
+                    0, Overlapped),
+                StorageUnit->Pipe, Overlapped, &BytesTransferred);
             if (ERROR_SUCCESS == Error)
             {
                 StorageUnit->Connected = -StorageUnit->Connected;
@@ -354,8 +350,8 @@ static DWORD SpdStorageUnitHandleTransactPipe(HANDLE Handle,
             memcpy(Msg + 1, DataBuffer, DataLength);
         Error = WaitOverlappedResult(
             StorageUnit->Event,
-            WriteFile(StorageUnit->Pipe, Msg, sizeof(TRANSACT_MSG) + DataLength, 0, &Overlapped),
-            StorageUnit->Pipe, &Overlapped, &BytesTransferred);
+            WriteFile(StorageUnit->Pipe, Msg, sizeof(TRANSACT_MSG) + DataLength, 0, Overlapped),
+            StorageUnit->Pipe, Overlapped, &BytesTransferred);
         if (ERROR_SUCCESS != Error)
             goto disconnect;
     }
@@ -365,8 +361,8 @@ static DWORD SpdStorageUnitHandleTransactPipe(HANDLE Handle,
         Error = WaitOverlappedResult(
             StorageUnit->Event,
             ReadFile(StorageUnit->Pipe,
-                Msg, sizeof(TRANSACT_MSG) + StorageUnit->StorageUnitParams.MaxTransferLength, 0, &Overlapped),
-            StorageUnit->Pipe, &Overlapped, &BytesTransferred);
+                Msg, sizeof(TRANSACT_MSG) + StorageUnit->StorageUnitParams.MaxTransferLength, 0, Overlapped),
+            StorageUnit->Pipe, Overlapped, &BytesTransferred);
         if (ERROR_SUCCESS != Error)
             goto disconnect;
 
@@ -418,8 +414,6 @@ static DWORD SpdStorageUnitHandleTransactPipe(HANDLE Handle,
 exit:
     MemFree(Msg);
 
-    SpdOverlappedFini(&Overlapped);
-
     return Error;
 
 disconnect:
@@ -544,12 +538,29 @@ DWORD SpdStorageUnitHandleTransact(HANDLE Handle,
     UINT32 Btl,
     SPD_IOCTL_TRANSACT_RSP *Rsp,
     SPD_IOCTL_TRANSACT_REQ *Req,
-    PVOID DataBuffer)
+    PVOID DataBuffer,
+    OVERLAPPED *Overlapped)
 {
+    OVERLAPPED TempOverlapped = { 0 };
+    DWORD Error;
+
+    if (Overlapped == NULL)
+    {
+        Error = SpdOverlappedInit(&TempOverlapped);
+        if (ERROR_SUCCESS != Error)
+            return Error;
+
+        Overlapped = &TempOverlapped;
+    }
+
     if (IsPipeHandle(Handle))
-        return SpdStorageUnitHandleTransactPipe(GetPipeHandle(Handle), Btl, Rsp, Req, DataBuffer);
+        Error = SpdStorageUnitHandleTransactPipe(GetPipeHandle(Handle), Btl, Rsp, Req, DataBuffer, Overlapped);
     else
-        return SpdIoctlTransact(GetDeviceHandle(Handle), Btl, Rsp, Req, DataBuffer);
+        Error = SpdIoctlTransact(GetDeviceHandle(Handle), Btl, Rsp, Req, DataBuffer, Overlapped);
+
+    SpdOverlappedFini(&TempOverlapped);
+
+    return Error;
 }
 
 DWORD SpdStorageUnitHandleShutdown(HANDLE Handle,

src/shared/stgunit.c

@@ -28,7 +28,8 @@ DWORD SpdStorageUnitHandleTransact(HANDLE Handle,
     UINT32 Btl,
     SPD_IOCTL_TRANSACT_RSP *Rsp,
     SPD_IOCTL_TRANSACT_REQ *Req,
-    PVOID DataBuffer);
+    PVOID DataBuffer,
+    OVERLAPPED *Overlapped);
 DWORD SpdStorageUnitHandleShutdown(HANDLE Handle,
     const GUID *Guid);
 DWORD SpdStorageUnitHandleClose(HANDLE Handle);
@@ -135,6 +136,7 @@ static DWORD WINAPI SpdStorageUnitDispatcherThread(PVOID StorageUnit0)
     SPD_IOCTL_TRANSACT_RSP ResponseBuf, *Response;
     SPD_STORAGE_UNIT_OPERATION_CONTEXT OperationContext;
     PVOID DataBuffer = 0;
+    OVERLAPPED Overlapped;
     HANDLE DispatcherThread = 0;
     BOOLEAN Complete;
     DWORD Error;
@@ -146,6 +148,10 @@ static DWORD WINAPI SpdStorageUnitDispatcherThread(PVOID StorageUnit0)
         goto exit;
     }
 
+    Error = SpdOverlappedInit(&Overlapped);
+    if (ERROR_SUCCESS != Error)
+        goto exit;
+
     OperationContext.Request = &RequestBuf;
     OperationContext.Response = &ResponseBuf;
     OperationContext.DataBuffer = DataBuffer;
@@ -165,9 +171,15 @@ static DWORD WINAPI SpdStorageUnitDispatcherThread(PVOID StorageUnit0)
     Response = 0;
     for (;;)
     {
+        if (!ResetEvent(Overlapped.hEvent))
+        {
+            Error = GetLastError();
+            goto exit;
+        }
+
         memset(Request, 0, sizeof *Request);
         Error = SpdStorageUnitHandleTransact(StorageUnit->Handle,
-            StorageUnit->Btl, Response, Request, DataBuffer);
+            StorageUnit->Btl, Response, Request, DataBuffer, &Overlapped);
         if (ERROR_SUCCESS != Error)
             goto exit;
 
@@ -277,6 +289,8 @@ static DWORD WINAPI SpdStorageUnitDispatcherThread(PVOID StorageUnit0)
 
     TlsSetValue(SpdStorageUnitTlsKey, 0);
 
+    SpdOverlappedFini(&Overlapped);
+
     StorageUnit->BufferFree(DataBuffer);
 
     return Error;
@@ -336,7 +350,7 @@ VOID SpdStorageUnitSendResponse(SPD_STORAGE_UNIT *StorageUnit,
     }
 
     Error = SpdStorageUnitHandleTransact(StorageUnit->Handle,
-        StorageUnit->Btl, Response, 0, DataBuffer);
+        StorageUnit->Btl, Response, 0, DataBuffer, NULL);
     if (ERROR_SUCCESS != Error)
     {
         SpdStorageUnitSetDispatcherError(StorageUnit, Error);