• ExAcquireFastMutex, in the last code block. This raises IRQL to APC.

    Sympathies for such a legacy ...

    -- pa  
    Tuesday, January 8, 2019 11:24 PM

All replies

  • Are you calling the api while holding a spin lock? Did you enter a critical region or disable APCs before making the API call?

    d -- This posting is provided "AS IS" with no warranties, and confers no rights.

    Tuesday, January 8, 2019 9:45 PM
  • As Doron stated, there are a lot of ways to raise IRQL.   Did you run code analysis on this (my preference is turn on all rules) and fix the warnings, this is a simple (though not perfect) way of finding a lot of these problems.

    Don Burn Windows Driver Consulting Website:

    Tuesday, January 8, 2019 10:37 PM
  • No. I inherited this code.  Do not see any spin locks or critical sections. Driver installs and runs fine in Windows 7.  Driver installed and ran fine in Windows 10 until root certificate problem. Bug check occurs after attestation signing.

    Kernel calls AddDevice().  AddDevice() calls InitialiazeGenericExtension().  InitializeGenericExtension() calls GenericRegisterInterface().  GenericRegisterInterface() calls IoRegisterDeviceInterface().  All code below.

        IN PDRIVER_OBJECT DriverObject, 
    {   // AddDevice(): 
        //  - Symmetric with RemoveDevice()
        PDEVICE_OBJECT fdo;
        HISTO(DEFAULT_HISTORY, "AddD", 0, 0, 0);
        // Device extension (with extensions to the extension)
        //  - Total device extension includes other private extensions:
        ULONG xsize = (sizeof(DEVICE_EXTENSION) + 7) & ~7;
        ULONG gsize = GetSizeofGenericExtension();      // For Oney's "Generic" (optional, recommended)
        // Create a functional device object to represent the hardware we're managing.
        NTSTATUS NtStatus = IoCreateDevice(
            DriverObject,               // IN PDRIVER_OBJECT  DriverObject,
            xsize + gsize,              // IN ULONG  DeviceExtensionSize,
            NULL,                       // IN PUNICODE_STRING  DeviceName  OPTIONAL,
            FILE_DEVICE_UNKNOWN,        // IN DEVICE_TYPE  DeviceType,
            //XXXXXXXXXXXXXX FILE_DEVICE_SECURE_OPEN,    // IN ULONG  DeviceCharacteristics,
            FILE_AUTOGENERATED_DEVICE_NAME,    // IN ULONG  DeviceCharacteristics,
            FALSE,                      // IN BOOLEAN  Exclusive,
            &fdo                        // OUT PDEVICE_OBJECT  *DeviceObject
        if (!NT_SUCCESS(NtStatus))
            DBG_PRINTF(VOLUME_MINIMUM, ("IoCreateDevice(): failed: NtStatus:%x\n", NtStatus));
            return NtStatus;
        PDEVICE_EXTENSION pdx = (PDEVICE_EXTENSION)fdo->DeviceExtension;
        DBG_PRINTF(VOLUME_LOW, ("dt "DRIVERNAME"!DEVICE_EXTENSION_ %p   ;* Device extension\n", pdx));
        // From this point forward, any error will have side effects that need to
        // be cleaned up. Using a try-finally block allows us to modify the program
        // easily without losing track of the side effects.
        for (;;)
            pdx->DeviceObject = fdo;
            pdx->Pdo = pdo;
            // Declare the buffering method we'll use for read/write requests
            fdo->Flags |= DO_DIRECT_IO;
            //XXXXXXXXXXXXXX BUGHUNT: User mode reads not working. OldPhoton appears to do both buffered and direct I/O!
            //XXXXXXXXXXXXXX fdo->Flags |= DO_BUFFERED_IO;
            // Link our device object into the stack leading to the PDO
            pdx->LowerDeviceObject = IoAttachDeviceToDeviceStack(fdo, pdo);
            if (!pdx->LowerDeviceObject)
                DBG_PRINTF(VOLUME_MINIMUM, ("IoAttachDeviceToDeviceStack(): failed\n"));
                NtStatus = STATUS_DEVICE_REMOVED;
            // Set power management flags in the device object
            fdo->Flags |= DO_POWER_PAGABLE;
            // PhGen: Important Notes:
            //  - PhGen is based on Walter Oney's "Generic" library. PhGen's usage
            //    is matches Oney's usage model, as shown in this code and 
            //    described in his books.
            //  - In this driver's context, PhGen handles PnP and power IRPs, but
            //    it can be used for more (for instance, IRP queuing).
            //  - Recommendation: Driver developers should have a copy of Walter 
            //    Oney's books: Please visit ****** HTTP://WWW.ONEYSOFT.COM/ ******
            // Initialize to use the PhGen library
            //  - In the memory layout, PhGen's private device extension follows 
            //    this driver's extension
            pdx->pgx = (PGENERIC_EXTENSION)((PUCHAR)pdx + xsize);
            GENERIC_INIT_STRUCT gis;
            RtlZeroMemory(&gis, sizeof(gis));
            gis.Size = sizeof(gis);
            gis.DeviceObject = fdo;
            gis.Pdo = pdo;
            gis.Ldo = pdx->LowerDeviceObject;
            gis.RemoveLock = &pdx->RemoveLock;
            // Plug and play callbacks
            //  - PhGen calls these functions as a result of associated PNP IRPs
            //  - See DispatchPnp()
            gis.StartDevice = StartDevice;
            gis.StopDevice = StopDevice;
            gis.RemoveDevice = RemoveDevice;
            gis.OkayToRemove = OkayToRemove;    // Called by PhGen in IRP_MN_QUERY_REMOVE_DEVICE handling
            // Power management callbacks
            //  - PhGen calls these functions as a result of associated power IRPs
            //  - See DispatchPower()
            gis.SaveDeviceContext = SaveDeviceContext;
            gis.RestoreDeviceContext = RestoreDeviceContext;
            RtlInitUnicodeString(&gis.DebugName, LDRIVERNAME);
            // Flags:
            // - GENERIC_SURPRISE_REMOVAL_OK manages the "Safely Remove Hardware"
            //   icon in the System Tray. Remove GENERIC_SURPRISE_REMOVAL_OK 
            //   from flags to show "Safely Remove Hardware" icon in System Tray
            // - Tip: See flags in PhGen.h:
            //    GENERIC_AUTOLAUNCH            // Register for AutoLaunch
            //    GENERIC_USAGE_PAGING          // Device supports DeviceUsageTypePaging
            //    GENERIC_USAGE_DUMP            // Device supports DeviceUsageTypeDumpFile
            //    GENERIC_USAGE_HIBERNATE       // Device supports DeviceUsageTypeHibernation
            //    GENERIC_PENDING_IOCTLS        // Driver may cache asynchronous IOCTLs
            //    GENERIC_SURPRISE_REMOVAL_OK   // Surprise removal of device is okay
            //    GENERIC_IDLE_DETECT           // Device supports generic idle detection scheme
            gis.Flags = GENERIC_SURPRISE_REMOVAL_OK;
            NtStatus = InitializeGenericExtension(pdx->pgx, &gis);
            if (!NT_SUCCESS(NtStatus))
                DBG_PRINTF(VOLUME_MINIMUM, ("InitializeGenericExtension(): Failed: NtStatus:%x\n", NtStatus));
            // Exposes user-mode interface GUID:
            //  - Your DLL or app references your registered PnP GUID for device 
            //    arrival/removal notification
            //  - Your user-mode app or DLL must use the same GUID value. The GUID
            //    is therefore often defined in a file shared by both this
            //    driver and its user-mode counterpart.
            //  - Use GuidGen.EXE (from MS-SDK) to generate a new, unique GUID.
            //  - See MSDN for information on GUIDs and user-mode PnP support: 
            //     - SetupDiGetClassDevs()
            //     - SetupDiEnumDeviceInterfaces()
            //     - SetupDiGetDeviceInterfaceDetail()
            //     - WM_DEVICECHANGE
            // Suggestion: Use Oney's 'generic' interface registration technique:
            DBG_PRINTF(VOLUME_NONE, ("Photon's interface GUID:    {%8.8x-%4.4x-%4.4x-%2.2x%2.2x-%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x}\n", 
                AddDevice_GuidPhoton->Data1,    // 1 ulong, then hyphen (01234567-)
                AddDevice_GuidPhoton->Data2,    // 1 short, then hyphen (abcd-)
                AddDevice_GuidPhoton->Data3,    // 1 short, then hyphen (0123-)
                AddDevice_GuidPhoton->Data4[0], // 2 bytes, then hyphen (abcd-)
                AddDevice_GuidPhoton->Data4[2], // 6 bytes, then done (0123456789ab)
            NtStatus = GenericRegisterInterface(pdx->pgx, AddDevice_GuidPhoton);
            ASSERTMSG("AddDevice(): GenericRegisterInterface() failed\n", NtStatus == STATUS_SUCCESS); //XXXXXXXXXXXXBUGHUNT: User mode reads not working. 
            GenericEnableInterface(pdx->pgx, AddDevice_GuidPhoton, TRUE);                              //XXXXXXXXXXXXBUGHUNT: User mode reads not working. Do we need a matching Disable somewhere?
            ASSERTMSG("AddDevice(): GenericEnableInterface() failed\n", NtStatus == STATUS_SUCCESS);   //XXXXXXXXXXXXBUGHUNT: User mode reads not working. 
    #if DBG
            // Register Commander's GUID
            //  - Commander applies to a special development user-mode application
            DBG_PRINTF(VOLUME_NONE, ("Commander's interface GUID: {%8.8x-%4.4x-%4.4x-%2.2x%2.2x-%2.2x%2.2x%2.2x%2.2x%2.2x%2.2x} \n", 
                AddDevice_GuidCommander->Data1,     // 1 ulong, then hyphen (01234567-)
                AddDevice_GuidCommander->Data2,     // 1 short, then hyphen (abcd-)
                AddDevice_GuidCommander->Data3,     // 1 short, then hyphen (0123-)
                AddDevice_GuidCommander->Data4[0],  // 2 bytes, then hyphen (abcd-)
                AddDevice_GuidCommander->Data4[2],  // 6 bytes, then done (0123456789ab)
            GenericRegisterInterface(pdx->pgx, AddDevice_GuidCommander);
            // Clear the "initializing" flag so that we can get IRPs
            fdo->Flags &= ~DO_DEVICE_INITIALIZING;
            // Set MS MaxTransferSize
            //   - Specifies the largest size of a transfer for a single URB
            //   - For MSDN information on MaxTransferSize, see:
            /*XXXXXXXXXXXX Vista: I don't know what IoIsWdmVersionAvailable() returns!
            if (IoIsWdmVersionAvailable(1, ???))
            {   // Vista or above
                //XXXXXXXXX OHCI or UHCI restrictions?
                prx->MsMaximumTransferSize = 0x370000;
            else if ...
            if (IoIsWdmVersionAvailable(1, 0x10))
            {   // Win2k or above
                //  - Above Win2K, MS-USBD ignores MaximumTransferSize
                pdx->MsMaximumTransferSize = 0x40000;
            {   // WinME, Win98, ...
                //  - Older OS's aren't really even supported!
                pdx->MsMaximumTransferSize = 0x10000;
        if (!NT_SUCCESS(NtStatus))
            if (pdx->LowerDeviceObject)
        KeInitializeMutex(&pdx->Mutex, 0);  // Oney: "...kernel currently ignores the level [arg2] parameter"
        return NtStatus;
        {                           // InitializeGenericExtension
        if (isp->Size < FIELD_OFFSET(GENERIC_INIT_STRUCT, Flags)
            || !isp->DeviceObject
            || !isp->Ldo
            || !isp->Pdo
            || !isp->StartDevice
            || !isp->StopDevice
            || !isp->RemoveDevice
            || isp->DeviceQueue && !isp->StartIo)
            return STATUS_INVALID_PARAMETER;
        RtlZeroMemory(pdx, sizeof(GENERIC_EXTENSION));
        pdx->DeviceObject = isp->DeviceObject;
        pdx->LowerDeviceObject = isp->Ldo;
        pdx->Pdo = isp->Pdo;
        pdx->StartDevice = isp->StartDevice;
        pdx->StopDevice = isp->StopDevice;
        pdx->RemoveDevice = isp->RemoveDevice;
        if (isp->Size >= FIELD_OFFSET(GENERIC_INIT_STRUCT, OkayToRemove) + sizeof(PQUERYFUNCTION))
            {                       // set OkayToStop & OkayToRemove pointers
            pdx->OkayToStop = isp->OkayToStop;
            pdx->OkayToRemove = isp->OkayToRemove;
            }                       // set OkayToStop & OkayToRemove pointers
        if ((pdx->RemoveLock = isp->RemoveLock))
            IoInitializeRemoveLock(pdx->RemoveLock, 0, 0, 0);
        pdx->state = STOPPED;
        pdx->devpower = PowerDeviceD0;
        pdx->syspower = PowerSystemWorking;
        POWER_STATE state;
        state.DeviceState = PowerDeviceD0;
        PoSetPowerState(pdx->DeviceObject, DevicePowerState, state);
        // In version 1.3, I added support for multiple IRP queues
        if (isp->Size >= FIELD_OFFSET(GENERIC_INIT_STRUCT, NumberOfQueues) + sizeof(ULONG)
            && isp->NumberOfQueues)
            {                       // multiple queues
            if (isp->DeviceQueue || isp->StartIo)
                return STATUS_INVALID_PARAMETER;    // can't mix new and old ways of identifying queues
            if (isp->Size < FIELD_OFFSET(GENERIC_INIT_STRUCT, Queues) + isp->NumberOfQueues * 2 * sizeof(PVOID))
                return STATUS_INVALID_PARAMETER;    // init structure not big enough
            for (ULONG i = 0; i < isp->NumberOfQueues; ++i)
                if (!isp->Queues[i].DeviceQueue || !isp->Queues[i].StartIo)
                    return STATUS_INVALID_PARAMETER;    // none of the entries can be NULL
            pdx->nqueues = isp->NumberOfQueues;
            pdx->queues = (PDEVQUEUE*) ExAllocatePoolWithTag(NonPagedPool, isp->NumberOfQueues * sizeof(PDEVQUEUE), '1inI');
            if (!pdx->queues)
            for (ULONG i = 0; i < isp->NumberOfQueues; ++i)
                {                   // for each queue
                pdx->queues[i] = isp->Queues[i].DeviceQueue;
                InitializeQueue(pdx->queues[i], isp->Queues[i].StartIo);
                }                   // for each queue
            }                       // multiple queues
        else if (isp->DeviceQueue)
            {                       // single queue
            pdx->nqueues = 1;
            pdx->queues = (PDEVQUEUE*) ExAllocatePoolWithTag(NonPagedPool, sizeof(PDEVQUEUE), '2inI');
            if (!pdx->queues)
            pdx->queues[0] = isp->DeviceQueue;
            InitializeQueue(pdx->queues[0], isp->StartIo);
            }                       // single queue
        // Capture the mini-driver name for messages. This needs to be in ANSI because
        // unicode conversions at or above DISPATCH_LEVEL are not allowed
        if (!isp->DebugName.Length)
            strcpy(pdx->DebugName, "GENERIC");
            {                       // convert debug name
            ANSI_STRING asname = {0, sizeof(pdx->DebugName) - 1, pdx->DebugName};
            RtlUnicodeStringToAnsiString(&asname, &isp->DebugName, FALSE);
            pdx->DebugName[asname.Length] = 0;
            }                       // convert debug name
        if (isp->Size >= FIELD_OFFSET(GENERIC_INIT_STRUCT, Flags) + sizeof(ULONG))
            pdx->Flags = isp->Flags & GENERIC_CLIENT_FLAGS;
        if (isp->Size >= FIELD_OFFSET(GENERIC_INIT_STRUCT, RestoreDeviceContext) + sizeof(PCONTEXTFUNCTION))
            {                       // get power helper functions
            pdx->QueryPower = isp->QueryPower;
            pdx->SaveDeviceContext = isp->SaveDeviceContext;
            pdx->RestoreDeviceContext = isp->RestoreDeviceContext;
            }                       // get power helper functions
        if (isp->Size >= FIELD_OFFSET(GENERIC_INIT_STRUCT, PerfBoundary) + sizeof(DEVICE_POWER_STATE))
            pdx->PerfBoundary = isp->PerfBoundary;
            pdx->PerfBoundary = PowerDeviceUnspecified;
        if (pdx->PerfBoundary == PowerDeviceUnspecified)
            pdx->PerfBoundary = PowerDeviceMaximum; // inhibit POWER_SEQUENCE optimization
        // Initialize variables related to asynchrounous IOCTL management.
        if (pdx->Flags & GENERIC_PENDING_IOCTLS)
            {                       // driver may cache asyncronous IOCTLs
            pdx->IoctlAbortStatus = 0;
            // We need to initialize our IOCTL spin lock sometime, but just once.
            // Acquiring the cancel spin lock to guard this operation is a bit of
            // a hack, I suppose, but note that a class driver like this one never
            // gets an actual chance to initialize, so it's not my fault...
            KIRQL oldirql;
            IoAcquireCancelSpinLock(&oldirql);  // any global spin lock would do
            if (!IoctlListLockInitialized)
                {                   // initialize global lock
                IoctlListLockInitialized = TRUE;
                }                   // initialize global lock
            }                       // driver may cache asynchronous IOCTLs
        // Initialize to manage registered device interfaces
        // Indicate we handle power IRPs at PASSIVE_LEVEL
        pdx->DeviceObject->Flags |= DO_POWER_PAGABLE;
        KdPrint(("GENERIC - Initializing for %s\n", pdx->DebugName));
        // If device honors paging-path notifications, initialize a synchronization
        // event in the signalled state to act as a simple mutex (SP-7)
        if (pdx->Flags & GENERIC_USAGE_PAGING)
            KeInitializeEvent(&pdx->evPagingPath, SynchronizationEvent, TRUE);
        // If requested to do so, register an AutoLaunch interface
        if (pdx->Flags & GENERIC_AUTOLAUNCH)
            GenericRegisterInterface(pdx, &GUID_AUTOLAUNCH_NOTIFY);
        // Register a power management interface
        GenericRegisterInterface(pdx, &GUID_GENERIC_POWER);
    #ifdef _X86_
        win98 = IsWin98();
        return STATUS_SUCCESS;
        }                           // InitializeGenericExtension
        {                           // GenericRegisterInterface
        NTSTATUS status;
        PINTERFACE_RECORD ifp = FindInterfaceRecord(pdx, guid);
        if (ifp)
            status = STATUS_INVALID_PARAMETER;
            {                       // register new interface
            ifp = (PINTERFACE_RECORD) ExAllocatePoolWithTag(NonPagedPool, sizeof(INTERFACE_RECORD), ' geR');
            if (ifp)
                {                   // initialize new interface record
                status = IoRegisterDeviceInterface(pdx->Pdo, guid, NULL, &ifp->linkname);
                if (NT_SUCCESS(status))
                    {               // interface registered
                    ifp->guid = *guid;
                    InsertHeadList(&pdx->iflist, &ifp->list);
                    }               // interface registered
                }                   // initialize new interface record
                status = STATUS_INSUFFICIENT_RESOURCES;
            }                       // register new interface
        return status;
        }                           // GenericRegisterInterface

    Tuesday, January 8, 2019 10:41 PM
  • ExAcquireFastMutex, in the last code block. This raises IRQL to APC.

    Sympathies for such a legacy ...

    -- pa  
    Tuesday, January 8, 2019 11:24 PM
  • No other thread/context can touch the list head in the device extension while you are in AddDevice.  When calling this from AddDevice just skip the acquisition of the fast mutex.

    d -- This posting is provided "AS IS" with no warranties, and confers no rights.

    Tuesday, January 8, 2019 11:32 PM
  • Pavel,

    Let me thank you for your time and expertise.  Your prompt response and professionalism are exceptional.  Thank you SO MUCH for your help! I am trying your solution now - and will probably be back soon with another bug check...:(

    A new corporate master changed my email and makes me look like a newcomer.  But, I have been using MSDN since early 90's, and the forums since they were formed. In the 25 years I have been part of MSDN this is the BEST I have ever experienced.  Thank you again!


    Wednesday, January 9, 2019 3:21 PM
  • Kurt,

        Run the Code Analysis on the driver, the bug Pavel identified would have been flagged by the tool.   It is better to let the tool find the tool find the easy ones, and you deal with the really challenging ones.

        The other thing to consider is as Pavel gave his condolences about the legacy code, is how complex is the "meat of the driver", i.e. the code that actually does the operations that the device needs to do?   This code has enough challenges it may be better to identify the meat such as the read/write/IOCTL paths and upgrade the whole driver to KMDF.

    Don Burn Windows Driver Consulting Website:

    Wednesday, January 9, 2019 3:29 PM
  • We inherited the device and driver.  Redesigned the device but kept the driver. Back in the day, I upgraded our PCI board driver to WDM back in the Windows 95 days.  That makes me the 'driver expert'...but I haven't done anything substantial for 10 years.

    We still build this driver using WinDDK 7600.16385.1 command line build environment. Until late last year, the driver we built in 2012 worked just fine.

    Since this is a simple USB device, our plan is to use WinUSB. But that software won't be done until later this year. In the meantime, Windows 10 threw us a curve requiring signing through the Dev Portal. Hopefully this 'fix' will see us through the rest of the driver's useful life...

    Am VERY happy to report that it now installs using our freshly minted EV certificate and runs after commenting ALL of the ExAcquireFastMutex/ExReleaseFastMutex pairs.  Thanks again.  All of you have been most gracious.

    Wednesday, January 9, 2019 5:42 PM
  • Hope you all are still watching...

    Tried the new driver installation on Windows 7 - "Windows can’t verify the publisher of this driver software"

    So..are we really saying that a driver EV and cross signed for Windows 10 will not install on Windows 7?


    Wednesday, January 9, 2019 9:23 PM
  • Really.

    -- pa

    Thursday, January 10, 2019 2:41 AM
  • Removing all of the ExAcquireFastMutex/ExReleaseFastMutex pairs around the device interface list is only safe if you are

    1) only adding the entries in AddDevice

    2) only removing them in IRP_MN_REMOVE_DEVICE AFTER you have made sure all of thread contexts (DPCs, work items, etc) which can touch the list have been run down and exited

    otherwise, you have just introduced a potential race between add and remove and thus memory corruptor/BSOD

    d -- This posting is provided "AS IS" with no warranties, and confers no rights.

    Thursday, January 10, 2019 6:48 PM