Merge pull request #121 from wm1/usb/kmdf_fx2/codeanalysis

Usb/kmdf fx2/codeanalysis

5 files changed, 84 insertions, 70 deletions

diff --git a/usb/kmdf_fx2/driver/Device.c b/usb/kmdf_fx2/driver/Device.c
index 34aff070..0f3d1349 100644
--- a/usb/kmdf_fx2/driver/Device.c
+++ b/usb/kmdf_fx2/driver/Device.c
@@ -165,22 +165,22 @@ Return Value:
                              WdfIoQueueDispatchParallel);
 
     ioQueueConfig.EvtIoDeviceControl    = OsrFxEvtIoDeviceControl;
-    
+
     //
-    // By default, Static Driver Verifier (SDV) displays a warning if it 
-    // doesn't find the EvtIoStop callback on a power-managed queue. 
-    // The 'assume' below causes SDV to suppress this warning. If the driver 
+    // By default, Static Driver Verifier (SDV) displays a warning if it
+    // doesn't find the EvtIoStop callback on a power-managed queue.
+    // The 'assume' below causes SDV to suppress this warning. If the driver
     // has not explicitly set PowerManaged to WdfFalse, the framework creates
-    // power-managed queues when the device is not a filter driver.  Normally 
+    // power-managed queues when the device is not a filter driver.  Normally
     // the EvtIoStop is required for power-managed queues, but for this driver
-    // it is not needed b/c the driver doesn't hold on to the requests for 
-    // long time or forward them to other drivers. 
+    // it is not needed b/c the driver doesn't hold on to the requests for
+    // long time or forward them to other drivers.
     // If the EvtIoStop callback is not implemented, the framework waits for
-    // all driver-owned requests to be done before moving in the Dx/sleep 
-    // states or before removing the device, which is the correct behavior 
+    // all driver-owned requests to be done before moving in the Dx/sleep
+    // states or before removing the device, which is the correct behavior
     // for this type of driver. If the requests were taking an indeterminate
     // amount of time to complete, or if the driver forwarded the requests
-    // to a lower driver/another stack, the queue should have an 
+    // to a lower driver/another stack, the queue should have an
     // EvtIoStop/EvtIoResume.
     //
     __analysis_assume(ioQueueConfig.EvtIoStop != 0);
@@ -189,7 +189,7 @@ Return Value:
                              WDF_NO_OBJECT_ATTRIBUTES,
                              &queue);// pointer to default queue
     __analysis_assume(ioQueueConfig.EvtIoStop == 0);
-    
+
     if (!NT_SUCCESS(status)) {
         TraceEvents(TRACE_LEVEL_ERROR, DBG_PNP,
                             "WdfIoQueueCreate failed  %!STATUS!\n", status);
@@ -308,9 +308,9 @@ Return Value:
         goto Error;
     }
 
-    // 
-    // Create the lock that we use to serialize calls to ResetDevice(). As an 
-    // alternative to using a WDFWAITLOCK to serialize the calls, a sequential 
+    //
+    // Create the lock that we use to serialize calls to ResetDevice(). As an
+    // alternative to using a WDFWAITLOCK to serialize the calls, a sequential
     // WDFQUEUE can be created and reset IOCTLs would be forwarded to it.
     //
     WDF_OBJECT_ATTRIBUTES_INIT(&attributes);
@@ -368,7 +368,7 @@ Return Value:
                      "IoSetDeviceInterfacePropertyData failed to set restricted property  %!STATUS!\n", status);
             goto Error;
         }
-#if (NTDDI_VERSION >= NTDDI_WIN10_RS2)
+#if defined(NTDDI_WIN10_RS2) && (NTDDI_VERSION >= NTDDI_WIN10_RS2)
 
         //
         // Adding Custom Capability:
@@ -494,7 +494,7 @@ Return Value:
                  "WdfUsbTargetDeviceCreateWithParameters failed with Status code %!STATUS!\n", status);
             return status;
         }
- 
+
         //
         // TODO: If you are fetching configuration descriptor from device for
         // selecting a configuration or to parse other descriptors, call OsrFxValidateConfigurationDescriptor
@@ -767,7 +767,7 @@ Return Value:
     // USBD_ValidateConfigurationDescriptor validates that all descriptors are completely contained within the configuration descriptor buffer.
     // It also checks for interface numbers, number of endpoints in an interface etc.
     // Please refer to msdn documentation for this function for more information.
-    //   
+    //
 
     status = USBD_ValidateConfigurationDescriptor( ConfigDesc, BufferLength , ValidationLevel , Offset , POOL_TAG );
     if (!(NT_SUCCESS (status)) ){
@@ -776,10 +776,10 @@ Return Value:
 
     //
     // TODO: You should validate the correctness of other descriptors which are not taken care by USBD_ValidateConfigurationDescriptor
-    // Check that all such descriptors have size >= sizeof(the descriptor they point to) 
-    // Check for any association between them if required 
-    // 
-   
+    // Check that all such descriptors have size >= sizeof(the descriptor they point to)
+    // Check for any association between them if required
+    //
+
     return status;
 }
 
diff --git a/usb/kmdf_fx2/driver/driver.c b/usb/kmdf_fx2/driver/driver.c
index 44939a3b..18905851 100644
--- a/usb/kmdf_fx2/driver/driver.c
+++ b/usb/kmdf_fx2/driver/driver.c
@@ -132,7 +132,7 @@ Return Value:
 
     //
     // Register with ETW (unified tracing)
-    // 
+    //
     EventRegisterOSRUSBFX2();
 
     //
@@ -207,6 +207,11 @@ Return Value:
 
 --*/
 {
+    //
+    // EvtCleanupCallback for WDFDRIVER is always called at PASSIVE_LEVEL
+    //
+    _Analysis_assume_(KeGetCurrentIrql() == PASSIVE_LEVEL);
+
     PAGED_CODE ();
 
     TraceEvents(TRACE_LEVEL_INFORMATION, DBG_INIT,
@@ -215,7 +220,7 @@ Return Value:
     WPP_CLEANUP( WdfDriverWdmGetDriverObject( (WDFDRIVER)Driver ));
 
     UNREFERENCED_PARAMETER(Driver); // For the case when WPP is not being used.
-	
+
     EventUnregisterOSRUSBFX2();
 }
 
diff --git a/usb/kmdf_fx2/driver/ioctl.c b/usb/kmdf_fx2/driver/ioctl.c
index 0f29dc8c..2cb4fcb1 100644
--- a/usb/kmdf_fx2/driver/ioctl.c
+++ b/usb/kmdf_fx2/driver/ioctl.c
@@ -43,7 +43,7 @@ OsrFxEvtIoDeviceControl(
     _In_ WDFREQUEST Request,
     _In_ size_t     OutputBufferLength,
     _In_ size_t     InputBufferLength,
-    _In_ ULONG      IoControlCode    
+    _In_ ULONG      IoControlCode
     )
 /*++
 
@@ -83,6 +83,12 @@ Return Value:
     UNREFERENCED_PARAMETER(InputBufferLength);
     UNREFERENCED_PARAMETER(OutputBufferLength);
 
+    //
+    // If your driver is at the top of its driver stack, EvtIoDeviceControl is called
+    // at IRQL = PASSIVE_LEVEL.
+    //
+    _Analysis_assume_(KeGetCurrentIrql() == PASSIVE_LEVEL);
+
     PAGED_CODE();
 
     TraceEvents(TRACE_LEVEL_INFORMATION, DBG_IOCTL, "--> OsrFxEvtIoDeviceControl\n");
@@ -384,9 +390,9 @@ StopAllPipes(
     )
 {
     WdfIoTargetStop(WdfUsbTargetPipeGetIoTarget(DeviceContext->InterruptPipe),
-                                 WdfIoTargetCancelSentIo);           
+                                 WdfIoTargetCancelSentIo);
     WdfIoTargetStop(WdfUsbTargetPipeGetIoTarget(DeviceContext->BulkReadPipe),
-                                 WdfIoTargetCancelSentIo);        
+                                 WdfIoTargetCancelSentIo);
     WdfIoTargetStop(WdfUsbTargetPipeGetIoTarget(DeviceContext->BulkWritePipe),
                                  WdfIoTargetCancelSentIo);
 }
@@ -440,9 +446,9 @@ Return Value:
 {
     PDEVICE_CONTEXT pDeviceContext;
     NTSTATUS status;
-    
+
     PAGED_CODE();
- 
+
     TraceEvents(TRACE_LEVEL_INFORMATION, DBG_IOCTL, "--> ResetDevice\n");
 
     pDeviceContext = GetDeviceContext(Device);
@@ -457,17 +463,17 @@ Return Value:
     }
 
     StopAllPipes(pDeviceContext);
-    
+
     status = WdfUsbTargetDeviceResetPortSynchronously(pDeviceContext->UsbDevice);
     if (!NT_SUCCESS(status)) {
         TraceEvents(TRACE_LEVEL_ERROR, DBG_IOCTL, "ResetDevice failed - 0x%x\n", status);
     }
-    
+
     status = StartAllPipes(pDeviceContext);
     if (!NT_SUCCESS(status)) {
         TraceEvents(TRACE_LEVEL_ERROR, DBG_IOCTL, "Failed to start all pipes - 0x%x\n", status);
     }
-    
+
     WdfWaitLockRelease(pDeviceContext->ResetDeviceWaitLock);
 
     TraceEvents(TRACE_LEVEL_INFORMATION, DBG_IOCTL, "<-- ResetDevice\n");
@@ -475,7 +481,7 @@ Return Value:
 }
 
 _IRQL_requires_(PASSIVE_LEVEL)
-NTSTATUS 
+NTSTATUS
 ReenumerateDevice(
     _In_ PDEVICE_CONTEXT DevContext
     )
@@ -499,7 +505,7 @@ Return Value:
     WDF_USB_CONTROL_SETUP_PACKET    controlSetupPacket;
     WDF_REQUEST_SEND_OPTIONS        sendOptions;
     GUID                            activity;
-    
+
     PAGED_CODE();
 
     TraceEvents(TRACE_LEVEL_VERBOSE, DBG_IOCTL,"--> ReenumerateDevice\n");
@@ -513,7 +519,7 @@ Return Value:
                                          &sendOptions,
                                          DEFAULT_CONTROL_TRANSFER_TIMEOUT
                                          );
-              
+
     WDF_USB_CONTROL_SETUP_PACKET_INIT_VENDOR(&controlSetupPacket,
                                         BmRequestHostToDevice,
                                         BmRequestToDevice,
@@ -546,15 +552,15 @@ Return Value:
                                  DevContext->DeviceName,
                                  DevContext->Location,
                                  status);
-   
+
     return status;
 
 }
 
 _IRQL_requires_(PASSIVE_LEVEL)
-NTSTATUS 
+NTSTATUS
 GetBarGraphState(
-    _In_ PDEVICE_CONTEXT DevContext, 
+    _In_ PDEVICE_CONTEXT DevContext,
     _Out_ PBAR_GRAPH_STATE BarGraphState
     )
 /*++
@@ -638,9 +644,9 @@ Return Value:
 }
 
 _IRQL_requires_(PASSIVE_LEVEL)
-NTSTATUS 
+NTSTATUS
 SetBarGraphState(
-    _In_ PDEVICE_CONTEXT DevContext, 
+    _In_ PDEVICE_CONTEXT DevContext,
     _In_ PBAR_GRAPH_STATE BarGraphState
     )
 /*++
@@ -718,9 +724,9 @@ Return Value:
 }
 
 _IRQL_requires_(PASSIVE_LEVEL)
-NTSTATUS 
+NTSTATUS
 GetSevenSegmentState(
-    _In_ PDEVICE_CONTEXT DevContext, 
+    _In_ PDEVICE_CONTEXT DevContext,
     _Out_ PUCHAR SevenSegment
     )
 /*++
@@ -752,7 +758,7 @@ Return Value:
     NTSTATUS status;
     WDF_USB_CONTROL_SETUP_PACKET    controlSetupPacket;
     WDF_REQUEST_SEND_OPTIONS        sendOptions;
-    
+
     WDF_MEMORY_DESCRIPTOR memDesc;
     ULONG    bytesTransferred;
 
@@ -811,9 +817,9 @@ Return Value:
 }
 
 _IRQL_requires_(PASSIVE_LEVEL)
-NTSTATUS 
+NTSTATUS
 SetSevenSegmentState(
-    _In_ PDEVICE_CONTEXT DevContext, 
+    _In_ PDEVICE_CONTEXT DevContext,
     _In_ PUCHAR SevenSegment
     )
 /*++
@@ -892,9 +898,9 @@ Return Value:
 }
 
 _IRQL_requires_(PASSIVE_LEVEL)
-NTSTATUS 
+NTSTATUS
 GetSwitchState(
-    _In_ PDEVICE_CONTEXT DevContext, 
+    _In_ PDEVICE_CONTEXT DevContext,
     _In_ PSWITCH_STATE SwitchState
     )
 /*++
@@ -1034,11 +1040,11 @@ Return Value:
             }
 
             //
-            // Complete the request.  If we failed to get the output buffer then 
+            // Complete the request.  If we failed to get the output buffer then
             // complete with that status.  Otherwise complete with the status from the reader.
             //
-            WdfRequestCompleteWithInformation(request, 
-                                              NT_SUCCESS(status) ? ReaderStatus : status, 
+            WdfRequestCompleteWithInformation(request,
+                                              NT_SUCCESS(status) ? ReaderStatus : status,
                                               bytesReturned);
             status = STATUS_SUCCESS;
 
diff --git a/usb/kmdf_fx2/exe/testapp.c b/usb/kmdf_fx2/exe/testapp.c
index fce915b5..43b6c7a2 100644
--- a/usb/kmdf_fx2/exe/testapp.c
+++ b/usb/kmdf_fx2/exe/testapp.c
@@ -21,10 +21,10 @@ Environment:
 
 --*/
 
-            
+
 #include <DriverSpecs.h>
-_Analysis_mode_(_Analysis_code_type_user_code_) 
- 
+_Analysis_mode_(_Analysis_code_type_user_code_)
+
 #include <windows.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -117,6 +117,7 @@ GetDevicePath(
 
     deviceInterfaceList = (PWSTR)malloc(deviceInterfaceListLength * sizeof(WCHAR));
     if (deviceInterfaceList == NULL) {
+        bRet = FALSE;
         printf("Error allocating memory for device interface list.\n");
         goto clean0;
     }
@@ -158,6 +159,9 @@ clean0:
 }
 
 
+_Check_return_
+_Ret_notnull_
+_Success_(return != INVALID_HANDLE_VALUE)
 HANDLE
 OpenDevice(
     _In_ BOOL Synchronous
@@ -1201,10 +1205,9 @@ exit:
     }
 
     if (hWrite != INVALID_HANDLE_VALUE) {
+        _Analysis_assume_(hWrite != NULL);
         CloseHandle(hWrite);
     }
 
     return retValue;
 }
-
-
diff --git a/usb/umdf2_fx2/driver/Device.c b/usb/umdf2_fx2/driver/Device.c
index 8a6442a6..e7d521e3 100644
--- a/usb/umdf2_fx2/driver/Device.c
+++ b/usb/umdf2_fx2/driver/Device.c
@@ -161,22 +161,22 @@ Return Value:
                              WdfIoQueueDispatchParallel);
 
     ioQueueConfig.EvtIoDeviceControl    = OsrFxEvtIoDeviceControl;
-    
+
     //
-    // By default, Static Driver Verifier (SDV) displays a warning if it 
-    // doesn't find the EvtIoStop callback on a power-managed queue. 
-    // The 'assume' below causes SDV to suppress this warning. If the driver 
+    // By default, Static Driver Verifier (SDV) displays a warning if it
+    // doesn't find the EvtIoStop callback on a power-managed queue.
+    // The 'assume' below causes SDV to suppress this warning. If the driver
     // has not explicitly set PowerManaged to WdfFalse, the framework creates
-    // power-managed queues when the device is not a filter driver.  Normally 
+    // power-managed queues when the device is not a filter driver.  Normally
     // the EvtIoStop is required for power-managed queues, but for this driver
-    // it is not needed b/c the driver doesn't hold on to the requests for 
-    // long time or forward them to other drivers. 
+    // it is not needed b/c the driver doesn't hold on to the requests for
+    // long time or forward them to other drivers.
     // If the EvtIoStop callback is not implemented, the framework waits for
-    // all driver-owned requests to be done before moving in the Dx/sleep 
-    // states or before removing the device, which is the correct behavior 
+    // all driver-owned requests to be done before moving in the Dx/sleep
+    // states or before removing the device, which is the correct behavior
     // for this type of driver. If the requests were taking an indeterminate
     // amount of time to complete, or if the driver forwarded the requests
-    // to a lower driver/another stack, the queue should have an 
+    // to a lower driver/another stack, the queue should have an
     // EvtIoStop/EvtIoResume.
     //
     __analysis_assume(ioQueueConfig.EvtIoStop != 0);
@@ -185,7 +185,7 @@ Return Value:
                              WDF_NO_OBJECT_ATTRIBUTES,
                              &queue);// pointer to default queue
     __analysis_assume(ioQueueConfig.EvtIoStop == 0);
-    
+
     if (!NT_SUCCESS(status)) {
         TraceEvents(TRACE_LEVEL_ERROR, DBG_PNP,
                             "WdfIoQueueCreate failed  %!STATUS!\n", status);
@@ -304,7 +304,7 @@ Return Value:
         goto Error;
     }
 
-#if (NTDDI_VERSION >= NTDDI_WIN10_RS2)
+#if defined(NTDDI_WIN10_RS2) && (NTDDI_VERSION >= NTDDI_WIN10_RS2)
     //
     // Adding Custom Capability:
     //
@@ -335,9 +335,9 @@ Return Value:
     }
 #endif
 
-    // 
-    // Create the lock that we use to serialize calls to ResetDevice(). As an 
-    // alternative to using a WDFWAITLOCK to serialize the calls, a sequential 
+    //
+    // Create the lock that we use to serialize calls to ResetDevice(). As an
+    // alternative to using a WDFWAITLOCK to serialize the calls, a sequential
     // WDFQUEUE can be created and reset IOCTLs would be forwarded to it.
     //
     WDF_OBJECT_ATTRIBUTES_INIT(&attributes);
@@ -750,7 +750,7 @@ Return Value:
 
     configParams.Types.SingleInterface.ConfiguredUsbInterface =
         usbInterface;
-    
+
     configParams.Types.SingleInterface.NumberConfiguredPipes =
         WdfUsbInterfaceGetNumConfiguredPipes(usbInterface);