cwdm

WDM driver RemoveLockForward Rule


I want to apologize for how long this post is in advance. I wanted to provide as much info as possible so that everyone had the info needed to help answer my question.

I am trying to bring a driver that I inherited into compliance with Microsoft's standards. When I run the Standard Driver Verifier on the source code it says that it fails the RemoveLockForward rule.

The RemoveLockForward rule verifies that calls to IoAcquireRemoveLock and IoReleaseRemoveLock are used correctly when forwarding an IRP to another device.

Here is the source code in question:

1: NTSTATUS Cam_PnpAddDevice(IN PDRIVER_OBJECT DriverObject, IN PDEVICE_OBJECT PhysicalDeviceObject)
2: {
3:   NTSTATUS               ntStatus = STATUS_SUCCESS;
4:   PDEVICE_OBJECT         DeviceObject;
5:   PDEVICE_EXTENSION      deviceExtension;
6:   GUID                   camGUID;
7:   PCAMERA_STATE          cameraState;
8:   PNODE_DEVICE_EXTENSION pNodeExt;
9:   POWER_STATE            state;
10:
11:   ntStatus             = IoCreateDevice(DriverObject, sizeof(DEVICE_EXTENSION), NULL, FILE_DEVICE_UNKNOWN,0, FALSE, &DeviceObject);
12:   DeviceObject->Flags |= DO_DIRECT_IO;
13:   deviceExtension      = (PDEVICE_EXTENSION)DeviceObject->DeviceExtension;
14:   cameraState          = &(deviceExtension->CameraState);
15:   RtlZeroMemory(deviceExtension, sizeof(DEVICE_EXTENSION));
16:   deviceExtension->CSR_offset = 0xFFFFFFFF;
17:
18:   camGUID  = GUID_CAMERA;
19:   ntStatus = IoRegisterDeviceInterface(PhysicalDeviceObject, &camGUID, NULL, &deviceExtension->SymbolicLinkName);
20:   if (!NT_SUCCESS(ntStatus))
21:   {
22:     IoDeleteDevice(DeviceObject);
23:     ntStatus = STATUS_NO_SUCH_DEVICE;
24:     goto Exit_Cam_PnpAddDevice;
25:   }
26: 
27:   deviceExtension->StackDeviceObject = IoAttachDeviceToDeviceStack(DeviceObject, PhysicalDeviceObject);
28:   if (!deviceExtension->StackDeviceObject)
29:   {
30:     // Free up the symbolic link
31:     RtlFreeUnicodeString(&deviceExtension->SymbolicLinkName);
32: 
33:     IoDeleteDevice(DeviceObject);
34:     ntStatus = STATUS_NO_SUCH_DEVICE;
35:     goto Exit_Cam_PnpAddDevice;
36:   }
37:
38:   // Save the device object we created as our physical device object
39:   deviceExtension->PhysicalDeviceObject = DeviceObject;
40:
41:   // Get the port device object
42:   pNodeExt                          = PhysicalDeviceObject->DeviceExtension;
43:   deviceExtension->PortDeviceObject = pNodeExt->PortDeviceObject;
44:
45:   // Initialize power states
46:   deviceExtension->CurrentDevicePowerState.DeviceState = PowerDeviceD0;
47:   deviceExtension->CurrentSystemPowerState             = PowerSystemWorking;
48:
49:   state = deviceExtension->CurrentDevicePowerState;
50:   PoSetPowerState(DeviceObject, DevicePowerState, state);
51: 
52:   // Initialize the spinlock/list to store the bus reset irps...
53:   KeInitializeSpinLock(&deviceExtension->ResetSpinLock);
54:   KeInitializeSpinLock(&deviceExtension->CromSpinLock);
55:   KeInitializeSpinLock(&deviceExtension->AsyncSpinLock);
56:   KeInitializeSpinLock(&deviceExtension->IsochSpinLock);
57:   KeInitializeSpinLock(&deviceExtension->IsochResourceSpinLock);
58:   InitializeListHead(&deviceExtension->BusResetIrps);
59:   InitializeListHead(&deviceExtension->CromData);
60:   InitializeListHead(&deviceExtension->AsyncAddressData);
61:   InitializeListHead(&deviceExtension->IsochDetachData);
62:   InitializeListHead(&deviceExtension->IsochResourceData);
63:
64:   DeviceObject->Flags      &= DO_POWER_PAGABLE;
65:   DeviceObject->Flags      &= ~DO_DEVICE_INITIALIZING;
66:   cameraState->IsochChannel = (ULONG)-1;
67:
68:   // Free up the symbolic link
69:   RtlFreeUnicodeString(&deviceExtension->SymbolicLinkName);
70:
71:   IoInitializeRemoveLock(&deviceExtension->ioRemoveLock, (ULONG)'TCAM', 0, 0);
72:
73: Exit_Cam_PnpAddDevice:
74:   return (ntStatus);
75: }
76:
77:
78: NTSTATUS Cam_Pnp(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp)
79: {
80:   NTSTATUS           ntStatus        = STATUS_SUCCESS;
81:   PDEVICE_EXTENSION  deviceExtension = (PDEVICE_EXTENSION)DeviceObject->DeviceExtension;
82:   PIO_STACK_LOCATION IrpSp;
83:
84:   IrpSp = IoGetCurrentIrpStackLocation(Irp);
85:
86:   // This is the line that fails
87:   IoAcquireRemoveLock(&deviceExtension->ioRemoveLock, Irp);
88:   switch (IrpSp->MinorFunction)
89:   {
90:   case IRP_MN_STOP_DEVICE:
91:     ntStatus = Cam_PnpStopDevice(DeviceObject, Irp);
92:
93:     Irp->IoStatus.Status = STATUS_SUCCESS;
94:
95:     // Pass down to layer below us...
96:     ntStatus = Cam_SubmitIrpAsync(deviceExtension->StackDeviceObject, Irp, NULL);
97:     break;
98:   }
99:
100:   IoReleaseRemoveLock(&deviceExtension->ioRemoveLock, Irp);
101:   return (ntStatus);
102: }

According to the Static Driver Verifier:

1: Cam_PnpAddDevice
  3:   NTSTATUS ntStatus = STATUS_SUCCESS;
  11:   IoCreateDevice
  12:   DeviceObject->Flags |= DO_DIRECT_IO;
  13:   deviceExtension      = (PDEVICE_EXTENSION)DeviceObject->DeviceExtension;
  14:   cameraState          = &(deviceExtension->CameraState);
  15:   sdv_RtlZeroMemory
  16:   deviceExtension->CSR_offset = 0xFFFFFFFF;
  18:   camGUID  = GUID_CAMERA;
  19:   IoRegisterDeviceInterface
  20:   if (!NT_SUCCESS(ntStatus))
  27:   IoAttachDeviceToDeviceStack
  28:   if (!deviceExtension->StackDeviceObject)
  39:   deviceExtension->PhysicalDeviceObject = DeviceObject;
  42:   pNodeExt                          = PhysicalDeviceObject->DeviceExtension;
  43:   deviceExtension->PortDeviceObject = pNodeExt->PortDeviceObject;
  46:   deviceExtension->CurrentDevicePowerState.DeviceState = PowerDeviceD0;
  47:   deviceExtension->CurrentSystemPowerState             = PowerSystemWorking;
  49:   state = deviceExtension->CurrentDevicePowerState;
  50:   PoSetPowerState
  53:   sdv_KeInitializeSpinLock
  54:   sdv_KeInitializeSpinLock
  55:   sdv_KeInitializeSpinLock
  56:   sdv_KeInitializeSpinLock
  57:   sdv_KeInitializeSpinLock
  58:   InitializeListHead
  59:   InitializeListHead
  60:   InitializeListHead
  61:   InitializeListHead
  62:   InitializeListHead
  64:   DeviceObject->Flags      &= DO_POWER_PAGABLE;
  65:   DeviceObject->Flags      &= ~DO_DEVICE_INITIALIZING;
  66:   cameraState->IsochChannel = (ULONG)-1;
  69:   RtlFreeUnicodeString
  71:   sdv_IoInitializeRemoveLock
  74:   return (ntStatus);
...  // There is messages here from the SDV that I did not include because I didn't
...  // think they were relevant.  If anyone asks I will happily include them.
  78: Cam_Pnp
    80: NTSTATUS           ntStatus        = STATUS_SUCCESS;
    81: PDEVICE_EXTENSION  deviceExtension = DeviceObject->DeviceExtension;
    84: sdv_IoGetCurrentIrpStackLocation
    87: sdv_IoAcquireRemoveLock
      9645: switch (choice)
      9647: case 0: return STATUS_UNSUCCESSFUL;break;
      9659: SLIC_sdv_IoAcquireRemoveLock_exit
        82: if(LockDepth > 0 && $return!=STATUS_SUCCESS)
        86: else if($return==STATUS_SUCCESS)
      9659: Return

I would greatly appreciate if anyone can help me figure out where the problem is. I hope that I have provided enough info, but please let me know if there is anything else that I can provide that will help.


Solution

  • I found the answer:

    If IoAcquireRemoveLock returns a failure status, the driver should not continue processing the IRP. Instead, beginning with Windows Vista, the driver should call IoCompleteRequest to complete the IRP and then return the failure status.

    The Static Driver Verifier was first returning STATUS_UNSUCCESSFUL before returning STATUS_SUCCESS to make sure that both cases are handled correctly. I just needed a small update to the code to fix it.

    NTSTATUS Cam_Pnp(IN PDEVICE_OBJECT DeviceObject, IN PIRP Irp)
    {
      NTSTATUS           ntStatus        = STATUS_SUCCESS;
      PDEVICE_EXTENSION  deviceExtension = (PDEVICE_EXTENSION)DeviceObject->DeviceExtension;
      PIO_STACK_LOCATION IrpSp;
    
      IrpSp = IoGetCurrentIrpStackLocation(Irp);
    
      ntStatus = IoAcquireRemoveLock(&deviceExtension->ioRemoveLock, Irp);
      if (ntStatus != STATUS_SUCCESS)
      {
        IoCompleteRequest(Irp, IO_NO_INCREMENT);
        goto Exit_Cam_Pnp;
      }
    
      switch (IrpSp->MinorFunction)
      {
      case IRP_MN_STOP_DEVICE:
        ntStatus = Cam_PnpStopDevice(DeviceObject, Irp);
    
        Irp->IoStatus.Status = STATUS_SUCCESS;
    
        // Pass down to layer below us...
        ntStatus = Cam_SubmitIrpAsync(deviceExtension->StackDeviceObject, Irp, NULL);
        break;
      }
    
       IoReleaseRemoveLock(&deviceExtension->ioRemoveLock, Irp);
    Exit_Cam_Pnp:
       return (ntStatus);
    }