From 0e26222a74d18f915bb4dc6490e8ce971f12ebe3 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Sun, 18 May 2014 20:39:54 +0000 Subject: [PATCH] [NTOSKRNL] - Fail device initialization if a filter fails to load so the PnP manager can try again later - Fix some handle leaks - Reset device node flags after a remove IRP is sent [I8042PRT|MOUCLASS|KBDCLASS] - Implement proper support for PnP remove IRPs See issue #8238 for more details. svn path=/trunk/; revision=63360 --- reactos/drivers/input/i8042prt/pnp.c | 37 ++++++++++++++++++++ reactos/drivers/input/kbdclass/kbdclass.c | 14 ++++++-- reactos/drivers/input/mouclass/mouclass.c | 14 ++++++-- reactos/ntoskrnl/io/iomgr/driver.c | 41 +++++++++++++++++++---- reactos/ntoskrnl/io/pnpmgr/pnpinit.c | 22 ++++++++++-- reactos/ntoskrnl/io/pnpmgr/pnpmgr.c | 6 ++-- 6 files changed, 118 insertions(+), 16 deletions(-) diff --git a/reactos/drivers/input/i8042prt/pnp.c b/reactos/drivers/input/i8042prt/pnp.c index b0c67eccbed..fbc1753cbe0 100644 --- a/reactos/drivers/input/i8042prt/pnp.c +++ b/reactos/drivers/input/i8042prt/pnp.c @@ -644,6 +644,26 @@ i8042PnpStartDevice( return Status; } +static VOID +i8042RemoveDevice( + IN PDEVICE_OBJECT DeviceObject) +{ + PI8042_DRIVER_EXTENSION DriverExtension; + KIRQL OldIrql; + PFDO_DEVICE_EXTENSION DeviceExtension; + + DriverExtension = (PI8042_DRIVER_EXTENSION)IoGetDriverObjectExtension(DeviceObject->DriverObject, DeviceObject->DriverObject); + DeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension; + + KeAcquireSpinLock(&DriverExtension->DeviceListLock, &OldIrql); + RemoveEntryList(&DeviceExtension->ListEntry); + KeReleaseSpinLock(&DriverExtension->DeviceListLock, OldIrql); + + IoDetachDevice(DeviceExtension->LowerDevice); + + IoDeleteDevice(DeviceObject); +} + NTSTATUS NTAPI i8042Pnp( IN PDEVICE_OBJECT DeviceObject, @@ -710,6 +730,23 @@ i8042Pnp( TRACE_(I8042PRT, "IRP_MJ_PNP / IRP_MN_QUERY_PNP_DEVICE_STATE\n"); return ForwardIrpAndForget(DeviceObject, Irp); } + case IRP_MN_QUERY_REMOVE_DEVICE: + { + TRACE_(I8042PRT, "IRP_MJ_PNP / IRP_MN_QUERY_REMOVE_DEVICE\n"); + return ForwardIrpAndForget(DeviceObject, Irp); + } + case IRP_MN_CANCEL_REMOVE_DEVICE: + { + TRACE_(I8042PRT, "IRP_MJ_PNP / IRP_MN_CANCEL_REMOVE_DEVICE\n"); + return ForwardIrpAndForget(DeviceObject, Irp); + } + case IRP_MN_REMOVE_DEVICE: + { + TRACE_(I8042PRT, "IRP_MJ_PNP / IRP_MN_REMOVE_DEVICE\n"); + Status = ForwardIrpAndForget(DeviceObject, Irp); + i8042RemoveDevice(DeviceObject); + return Status; + } default: { ERR_(I8042PRT, "IRP_MJ_PNP / unknown minor function 0x%x\n", MinorFunction); diff --git a/reactos/drivers/input/kbdclass/kbdclass.c b/reactos/drivers/input/kbdclass/kbdclass.c index 12ff89e3fec..09967b9f749 100644 --- a/reactos/drivers/input/kbdclass/kbdclass.c +++ b/reactos/drivers/input/kbdclass/kbdclass.c @@ -606,7 +606,7 @@ DestroyPortDriver( /* Remove from ClassDeviceExtension->ListHead list */ KeAcquireSpinLock(&ClassDeviceExtension->ListSpinLock, &OldIrql); - RemoveHeadList(DeviceExtension->ListEntry.Blink); + RemoveEntryList(&DeviceExtension->ListEntry); KeReleaseSpinLock(&ClassDeviceExtension->ListSpinLock, OldIrql); /* Remove entry from HKEY_LOCAL_MACHINE\HARDWARE\DEVICEMAP\[DeviceBaseName] */ @@ -865,7 +865,6 @@ ClassPnp( IoCompleteRequest(Irp, IO_NO_INCREMENT); return Status; - case IRP_MN_REMOVE_DEVICE: case IRP_MN_STOP_DEVICE: if (DeviceExtension->FileHandle) { @@ -874,6 +873,17 @@ ClassPnp( } Status = STATUS_SUCCESS; break; + + case IRP_MN_REMOVE_DEVICE: + if (DeviceExtension->FileHandle) + { + ZwClose(DeviceExtension->FileHandle); + DeviceExtension->FileHandle = NULL; + } + IoSkipCurrentIrpStackLocation(Irp); + Status = IoCallDriver(DeviceExtension->LowerDevice, Irp); + DestroyPortDriver(DeviceObject); + return Status; default: Status = Irp->IoStatus.Status; diff --git a/reactos/drivers/input/mouclass/mouclass.c b/reactos/drivers/input/mouclass/mouclass.c index 070dc5096e4..6fb47b38db6 100644 --- a/reactos/drivers/input/mouclass/mouclass.c +++ b/reactos/drivers/input/mouclass/mouclass.c @@ -582,7 +582,7 @@ DestroyPortDriver( /* Remove from ClassDeviceExtension->ListHead list */ KeAcquireSpinLock(&ClassDeviceExtension->ListSpinLock, &OldIrql); - RemoveHeadList(DeviceExtension->ListEntry.Blink); + RemoveEntryList(&DeviceExtension->ListEntry); KeReleaseSpinLock(&ClassDeviceExtension->ListSpinLock, OldIrql); /* Remove entry from HKEY_LOCAL_MACHINE\HARDWARE\DEVICEMAP\[DeviceBaseName] */ @@ -841,7 +841,6 @@ ClassPnp( IoCompleteRequest(Irp, IO_NO_INCREMENT); return Status; - case IRP_MN_REMOVE_DEVICE: case IRP_MN_STOP_DEVICE: if (DeviceExtension->FileHandle) { @@ -850,6 +849,17 @@ ClassPnp( } Status = STATUS_SUCCESS; break; + + case IRP_MN_REMOVE_DEVICE: + if (DeviceExtension->FileHandle) + { + ZwClose(DeviceExtension->FileHandle); + DeviceExtension->FileHandle = NULL; + } + IoSkipCurrentIrpStackLocation(Irp); + Status = IoCallDriver(DeviceExtension->LowerDevice, Irp); + DestroyPortDriver(DeviceObject); + return Status; default: Status = Irp->IoStatus.Status; diff --git a/reactos/ntoskrnl/io/iomgr/driver.c b/reactos/ntoskrnl/io/iomgr/driver.c index 4cdbbb51484..105e8cad767 100644 --- a/reactos/ntoskrnl/io/iomgr/driver.c +++ b/reactos/ntoskrnl/io/iomgr/driver.c @@ -558,6 +558,10 @@ IopAttachFilterDriversCallback( PLDR_DATA_TABLE_ENTRY ModuleObject; PDRIVER_OBJECT DriverObject; NTSTATUS Status; + + /* No filter value present */ + if (ValueType == REG_NONE) + return STATUS_SUCCESS; for (Filters = ValueData; ((ULONG_PTR)Filters - (ULONG_PTR)ValueData) < ValueLength && @@ -578,18 +582,21 @@ IopAttachFilterDriversCallback( /* Load and initialize the filter driver */ Status = IopLoadServiceModule(&ServiceName, &ModuleObject); if (!NT_SUCCESS(Status)) - continue; + return Status; Status = IopInitializeDriverModule(DeviceNode, ModuleObject, &ServiceName, FALSE, &DriverObject); if (!NT_SUCCESS(Status)) - continue; + return Status; } Status = IopInitializeDevice(DeviceNode, DriverObject); /* Remove extra reference */ ObDereferenceObject(DriverObject); + + if (!NT_SUCCESS(Status)) + return Status; } return STATUS_SUCCESS; @@ -645,14 +652,23 @@ IopAttachFilterDrivers( QueryTable[0].Name = L"LowerFilters"; else QueryTable[0].Name = L"UpperFilters"; - QueryTable[0].Flags = RTL_QUERY_REGISTRY_REQUIRED; + QueryTable[0].Flags = 0; + QueryTable[0].DefaultType = REG_NONE; - RtlQueryRegistryValues( + Status = RtlQueryRegistryValues( RTL_REGISTRY_HANDLE, (PWSTR)SubKey, QueryTable, DeviceNode, NULL); + if (!NT_SUCCESS(Status)) + { + DPRINT1("Failed to load device %s filters: %08X\n", + Lower ? "lower" : "upper", Status); + ZwClose(SubKey); + ZwClose(EnumRootKey); + return Status; + } /* * Now get the class GUID @@ -696,9 +712,10 @@ IopAttachFilterDrivers( &Class, KEY_READ); if (!NT_SUCCESS(Status)) { + /* It's okay if there's no class key */ DPRINT1("ZwOpenKey() failed with Status %08X\n", Status); ZwClose(EnumRootKey); - return Status; + return STATUS_SUCCESS; } QueryTable[0].QueryRoutine = IopAttachFilterDriversCallback; @@ -707,9 +724,10 @@ IopAttachFilterDrivers( else QueryTable[0].Name = L"UpperFilters"; QueryTable[0].EntryContext = NULL; - QueryTable[0].Flags = RTL_QUERY_REGISTRY_REQUIRED; + QueryTable[0].Flags = 0; + QueryTable[0].DefaultType = REG_NONE; - RtlQueryRegistryValues( + Status = RtlQueryRegistryValues( RTL_REGISTRY_HANDLE, (PWSTR)SubKey, QueryTable, @@ -719,6 +737,15 @@ IopAttachFilterDrivers( /* Clean up */ ZwClose(SubKey); ZwClose(EnumRootKey); + + if (!NT_SUCCESS(Status)) + { + DPRINT1("Failed to load class %s filters: %08X\n", + Lower ? "lower" : "upper", Status); + ZwClose(SubKey); + ZwClose(EnumRootKey); + return Status; + } } return STATUS_SUCCESS; diff --git a/reactos/ntoskrnl/io/pnpmgr/pnpinit.c b/reactos/ntoskrnl/io/pnpmgr/pnpinit.c index 1684f92284c..30e20ca9e62 100644 --- a/reactos/ntoskrnl/io/pnpmgr/pnpinit.c +++ b/reactos/ntoskrnl/io/pnpmgr/pnpinit.c @@ -275,10 +275,10 @@ PipCallDriverAddDevice(IN PDEVICE_NODE DeviceNode, EnumRootKey, &DeviceNode->InstancePath, KEY_READ); + ZwClose(EnumRootKey); if (!NT_SUCCESS(Status)) { DPRINT1("IopOpenRegistryKeyEx() failed with Status %08X\n", Status); - ZwClose(EnumRootKey); return Status; } @@ -330,12 +330,17 @@ PipCallDriverAddDevice(IN PDEVICE_NODE DeviceNode, ClassKey, &Properties, KEY_READ); + ZwClose(ClassKey); if (!NT_SUCCESS(Status)) { /* No properties */ DPRINT("IopOpenRegistryKeyEx() failed with Status %08X\n", Status); PropertiesKey = NULL; } + else + { + ZwClose(PropertiesKey); + } } /* Free the registry data */ @@ -343,11 +348,22 @@ PipCallDriverAddDevice(IN PDEVICE_NODE DeviceNode, } /* Do ReactOS-style setup */ - IopAttachFilterDrivers(DeviceNode, TRUE); + Status = IopAttachFilterDrivers(DeviceNode, TRUE); + if (!NT_SUCCESS(Status)) + { + IopRemoveDevice(DeviceNode); + return Status; + } Status = IopInitializeDevice(DeviceNode, DriverObject); if (NT_SUCCESS(Status)) { - IopAttachFilterDrivers(DeviceNode, FALSE); + Status = IopAttachFilterDrivers(DeviceNode, FALSE); + if (!NT_SUCCESS(Status)) + { + IopRemoveDevice(DeviceNode); + return Status; + } + Status = IopStartDevice(DeviceNode); } diff --git a/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c b/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c index 5c18437a6f6..ee036a78770 100644 --- a/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c +++ b/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c @@ -580,7 +580,11 @@ IopSendRemoveDevice(IN PDEVICE_OBJECT DeviceObject) { IO_STACK_LOCATION Stack; PVOID Dummy; + PDEVICE_NODE DeviceNode = IopGetDeviceNode(DeviceObject); + /* Drop all our state for this device in case it isn't really going away */ + DeviceNode->Flags &= DNF_ENUMERATED | DNF_PROCESSED; + RtlZeroMemory(&Stack, sizeof(IO_STACK_LOCATION)); Stack.MajorFunction = IRP_MJ_PNP; Stack.MinorFunction = IRP_MN_REMOVE_DEVICE; @@ -4537,8 +4541,6 @@ IopPrepareDeviceForRemoval(IN PDEVICE_OBJECT DeviceObject, BOOLEAN Force) return Status; } - DeviceNode->Flags |= DNF_WILL_BE_REMOVED; - DeviceNode->Problem = CM_PROB_WILL_BE_REMOVED; if (DeviceRelations) IopSendRemoveDeviceRelations(DeviceRelations); IopSendRemoveChildDevices(DeviceNode); -- 2.17.1