[PATCH 1/5] wineusb.sys: Introduce new remove_pending_irps helper.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/wineusb.sys/wineusb.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/dlls/wineusb.sys/wineusb.c b/dlls/wineusb.sys/wineusb.c index b8cefdaff5e..8fb1d75f121 100644 --- a/dlls/wineusb.sys/wineusb.c +++ b/dlls/wineusb.sys/wineusb.c @@ -363,6 +363,20 @@ static NTSTATUS query_id(const struct usb_device *device, IRP *irp, BUS_QUERY_ID return STATUS_SUCCESS; } +static void remove_pending_irps(struct usb_device *device) +{ + LIST_ENTRY *entry; + IRP *irp; + + while ((entry = RemoveHeadList(&device->irp_list)) != &device->irp_list) + { + irp = CONTAINING_RECORD(entry, IRP, Tail.Overlay.ListEntry); + irp->IoStatus.Status = STATUS_DELETE_PENDING; + irp->IoStatus.Information = 0; + IoCompleteRequest(irp, IO_NO_INCREMENT); + } +} + static NTSTATUS pdo_pnp(DEVICE_OBJECT *device_obj, IRP *irp) { IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(irp); @@ -393,17 +407,8 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device_obj, IRP *irp) break; case IRP_MN_REMOVE_DEVICE: - { - LIST_ENTRY *entry; - EnterCriticalSection(&wineusb_cs); - while ((entry = RemoveHeadList(&device->irp_list)) != &device->irp_list) - { - irp = CONTAINING_RECORD(entry, IRP, Tail.Overlay.ListEntry); - irp->IoStatus.Status = STATUS_DELETE_PENDING; - irp->IoStatus.Information = 0; - IoCompleteRequest(irp, IO_NO_INCREMENT); - } + remove_pending_irps(device); LeaveCriticalSection(&wineusb_cs); if (device->removed) @@ -419,7 +424,6 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device_obj, IRP *irp) ret = STATUS_SUCCESS; break; - } default: FIXME("Unhandled minor function %#x.\n", stack->MinorFunction); -- 2.32.0
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/wineusb.sys/wineusb.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/dlls/wineusb.sys/wineusb.c b/dlls/wineusb.sys/wineusb.c index 8fb1d75f121..a72095409ff 100644 --- a/dlls/wineusb.sys/wineusb.c +++ b/dlls/wineusb.sys/wineusb.c @@ -145,7 +145,6 @@ static void remove_usb_device(libusb_device *libusb_device) if (device->libusb_device == libusb_device) { list_remove(&device->entry); - device->removed = TRUE; break; } } @@ -402,26 +401,24 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device_obj, IRP *irp) } case IRP_MN_START_DEVICE: - case IRP_MN_SURPRISE_REMOVAL: ret = STATUS_SUCCESS; break; - case IRP_MN_REMOVE_DEVICE: + case IRP_MN_SURPRISE_REMOVAL: EnterCriticalSection(&wineusb_cs); remove_pending_irps(device); + device->removed = TRUE; LeaveCriticalSection(&wineusb_cs); + ret = STATUS_SUCCESS; + break; - if (device->removed) - { - libusb_unref_device(device->libusb_device); - libusb_close(device->handle); + case IRP_MN_REMOVE_DEVICE: + remove_pending_irps(device); - irp->IoStatus.Status = STATUS_SUCCESS; - IoCompleteRequest(irp, IO_NO_INCREMENT); - IoDeleteDevice(device->device_obj); - return STATUS_SUCCESS; - } + libusb_unref_device(device->libusb_device); + libusb_close(device->handle); + IoDeleteDevice(device->device_obj); ret = STATUS_SUCCESS; break; -- 2.32.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/wineusb.sys/wineusb.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dlls/wineusb.sys/wineusb.c b/dlls/wineusb.sys/wineusb.c index a72095409ff..fae297915fc 100644 --- a/dlls/wineusb.sys/wineusb.c +++ b/dlls/wineusb.sys/wineusb.c @@ -726,9 +726,21 @@ static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device_obj, IRP *irp ULONG code = stack->Parameters.DeviceIoControl.IoControlCode; struct usb_device *device = device_obj->DeviceExtension; NTSTATUS status = STATUS_NOT_IMPLEMENTED; + BOOL removed; TRACE("device_obj %p, irp %p, code %#x.\n", device_obj, irp, code); + EnterCriticalSection(&wineusb_cs); + removed = device->removed; + LeaveCriticalSection(&wineusb_cs); + + if (removed) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + return STATUS_DELETE_PENDING; + } + switch (code) { case IOCTL_INTERNAL_USB_SUBMIT_URB: -- 2.32.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Handle IRP_MN_SURPRISE_REMOVAL and notify device thread to stop it, but only wait for it in IRP_MN_REMOVE_DEVICE, as it's probably waiting for an IRP sent to the lower driver, which needs to be notified of removal too first. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hidclass.sys/device.c | 39 ++++++++++++++++++++++++++++++++++++++ dlls/hidclass.sys/hid.h | 3 +++ dlls/hidclass.sys/pnp.c | 24 ++++++++++++++++++++++- 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c index 82366ad1888..d3f5fa90fc7 100644 --- a/dlls/hidclass.sys/device.c +++ b/dlls/hidclass.sys/device.c @@ -431,11 +431,24 @@ NTSTATUS WINAPI pdo_ioctl(DEVICE_OBJECT *device, IRP *irp) NTSTATUS rc = STATUS_SUCCESS; IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp ); BASE_DEVICE_EXTENSION *ext = device->DeviceExtension; + BOOL removed; + KIRQL irql; irp->IoStatus.Information = 0; TRACE("device %p ioctl(%x)\n", device, irpsp->Parameters.DeviceIoControl.IoControlCode); + KeAcquireSpinLock(&ext->u.pdo.lock, &irql); + removed = ext->u.pdo.removed; + KeReleaseSpinLock(&ext->u.pdo.lock, irql); + + if (removed) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + return STATUS_DELETE_PENDING; + } + switch (irpsp->Parameters.DeviceIoControl.IoControlCode) { case IOCTL_HID_GET_POLL_FREQUENCY_MSEC: @@ -588,6 +601,19 @@ NTSTATUS WINAPI pdo_read(DEVICE_OBJECT *device, IRP *irp) NTSTATUS rc = STATUS_SUCCESS; IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); int ptr = -1; + BOOL removed; + KIRQL irql; + + KeAcquireSpinLock(&ext->u.pdo.lock, &irql); + removed = ext->u.pdo.removed; + KeReleaseSpinLock(&ext->u.pdo.lock, irql); + + if (removed) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + return STATUS_DELETE_PENDING; + } packet = malloc(buffer_size); ptr = PtrToUlong( irp->Tail.Overlay.OriginalFileObject->FsContext ); @@ -662,7 +688,20 @@ NTSTATUS WINAPI pdo_write(DEVICE_OBJECT *device, IRP *irp) const WINE_HIDP_PREPARSED_DATA *data = ext->u.pdo.preparsed_data; HID_XFER_PACKET packet; ULONG max_len; + BOOL removed; NTSTATUS rc; + KIRQL irql; + + KeAcquireSpinLock(&ext->u.pdo.lock, &irql); + removed = ext->u.pdo.removed; + KeReleaseSpinLock(&ext->u.pdo.lock, irql); + + if (removed) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + return STATUS_DELETE_PENDING; + } irp->IoStatus.Information = 0; diff --git a/dlls/hidclass.sys/hid.h b/dlls/hidclass.sys/hid.h index 5a502840691..863bb9a8bd4 100644 --- a/dlls/hidclass.sys/hid.h +++ b/dlls/hidclass.sys/hid.h @@ -67,6 +67,9 @@ typedef struct _BASE_DEVICE_EXTENSION KSPIN_LOCK irp_queue_lock; LIST_ENTRY irp_queue; + KSPIN_LOCK lock; + BOOL removed; + BOOL is_mouse; UNICODE_STRING mouse_link_name; BOOL is_keyboard; diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index 5f59257cdf7..5d6fb3d068c 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -359,11 +359,24 @@ static NTSTATUS fdo_pnp(DEVICE_OBJECT *device, IRP *irp) } } +static void remove_pending_irps(BASE_DEVICE_EXTENSION *ext) +{ + IRP *irp; + + while ((irp = pop_irp_from_queue(ext))) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + } +} + static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) { IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); BASE_DEVICE_EXTENSION *ext = device->DeviceExtension; NTSTATUS status = irp->IoStatus.Status; + IRP *queued_irp; + KIRQL irql; TRACE("irp %p, minor function %#x.\n", irp, irpsp->MinorFunction); @@ -453,12 +466,14 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) IoSetDeviceInterfaceState(&ext->u.pdo.mouse_link_name, TRUE); if (ext->u.pdo.is_keyboard) IoSetDeviceInterfaceState(&ext->u.pdo.keyboard_link_name, TRUE); + + ext->u.pdo.removed = FALSE; status = STATUS_SUCCESS; break; case IRP_MN_REMOVE_DEVICE: { - IRP *queued_irp; + remove_pending_irps(ext); send_wm_input_device_change(ext, GIDC_REMOVAL); @@ -494,6 +509,13 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) } case IRP_MN_SURPRISE_REMOVAL: + KeAcquireSpinLock(&ext->u.pdo.lock, &irql); + ext->u.pdo.removed = TRUE; + KeReleaseSpinLock(&ext->u.pdo.lock, irql); + + remove_pending_irps(ext); + + SetEvent(ext->u.pdo.halt_event); status = STATUS_SUCCESS; break; -- 2.32.0
On 7/1/21 2:51 AM, Rémi Bernon wrote:
Handle IRP_MN_SURPRISE_REMOVAL and notify device thread to stop it, but only wait for it in IRP_MN_REMOVE_DEVICE, as it's probably waiting for an IRP sent to the lower driver, which needs to be notified of removal too first.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hidclass.sys/device.c | 39 ++++++++++++++++++++++++++++++++++++++ dlls/hidclass.sys/hid.h | 3 +++ dlls/hidclass.sys/pnp.c | 24 ++++++++++++++++++++++- 3 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c index 82366ad1888..d3f5fa90fc7 100644 --- a/dlls/hidclass.sys/device.c +++ b/dlls/hidclass.sys/device.c @@ -431,11 +431,24 @@ NTSTATUS WINAPI pdo_ioctl(DEVICE_OBJECT *device, IRP *irp) NTSTATUS rc = STATUS_SUCCESS; IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp ); BASE_DEVICE_EXTENSION *ext = device->DeviceExtension; + BOOL removed; + KIRQL irql;
irp->IoStatus.Information = 0;
TRACE("device %p ioctl(%x)\n", device, irpsp->Parameters.DeviceIoControl.IoControlCode);
+ KeAcquireSpinLock(&ext->u.pdo.lock, &irql); + removed = ext->u.pdo.removed; + KeReleaseSpinLock(&ext->u.pdo.lock, irql); + + if (removed) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + return STATUS_DELETE_PENDING; + } + switch (irpsp->Parameters.DeviceIoControl.IoControlCode) { case IOCTL_HID_GET_POLL_FREQUENCY_MSEC: @@ -588,6 +601,19 @@ NTSTATUS WINAPI pdo_read(DEVICE_OBJECT *device, IRP *irp) NTSTATUS rc = STATUS_SUCCESS; IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); int ptr = -1; + BOOL removed; + KIRQL irql; + + KeAcquireSpinLock(&ext->u.pdo.lock, &irql); + removed = ext->u.pdo.removed; + KeReleaseSpinLock(&ext->u.pdo.lock, irql); + + if (removed) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + return STATUS_DELETE_PENDING; + }
packet = malloc(buffer_size); ptr = PtrToUlong( irp->Tail.Overlay.OriginalFileObject->FsContext ); @@ -662,7 +688,20 @@ NTSTATUS WINAPI pdo_write(DEVICE_OBJECT *device, IRP *irp) const WINE_HIDP_PREPARSED_DATA *data = ext->u.pdo.preparsed_data; HID_XFER_PACKET packet; ULONG max_len; + BOOL removed; NTSTATUS rc; + KIRQL irql; + + KeAcquireSpinLock(&ext->u.pdo.lock, &irql); + removed = ext->u.pdo.removed; + KeReleaseSpinLock(&ext->u.pdo.lock, irql); + + if (removed) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + return STATUS_DELETE_PENDING; + }
irp->IoStatus.Information = 0;
diff --git a/dlls/hidclass.sys/hid.h b/dlls/hidclass.sys/hid.h index 5a502840691..863bb9a8bd4 100644 --- a/dlls/hidclass.sys/hid.h +++ b/dlls/hidclass.sys/hid.h @@ -67,6 +67,9 @@ typedef struct _BASE_DEVICE_EXTENSION KSPIN_LOCK irp_queue_lock; LIST_ENTRY irp_queue;
+ KSPIN_LOCK lock; + BOOL removed;
Style nitpick, but none of the other members of this structure are aligned...
+ BOOL is_mouse; UNICODE_STRING mouse_link_name; BOOL is_keyboard; diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index 5f59257cdf7..5d6fb3d068c 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -359,11 +359,24 @@ static NTSTATUS fdo_pnp(DEVICE_OBJECT *device, IRP *irp) } }
+static void remove_pending_irps(BASE_DEVICE_EXTENSION *ext) +{ + IRP *irp; + + while ((irp = pop_irp_from_queue(ext))) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + } +} + static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) { IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); BASE_DEVICE_EXTENSION *ext = device->DeviceExtension; NTSTATUS status = irp->IoStatus.Status; + IRP *queued_irp; + KIRQL irql;
TRACE("irp %p, minor function %#x.\n", irp, irpsp->MinorFunction);
@@ -453,12 +466,14 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) IoSetDeviceInterfaceState(&ext->u.pdo.mouse_link_name, TRUE); if (ext->u.pdo.is_keyboard) IoSetDeviceInterfaceState(&ext->u.pdo.keyboard_link_name, TRUE); + + ext->u.pdo.removed = FALSE; status = STATUS_SUCCESS; break;
case IRP_MN_REMOVE_DEVICE: { - IRP *queued_irp; + remove_pending_irps(ext);
In that case there's no reason to delete queued IRPs later in the handler.
send_wm_input_device_change(ext, GIDC_REMOVAL);
@@ -494,6 +509,13 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) }
case IRP_MN_SURPRISE_REMOVAL: + KeAcquireSpinLock(&ext->u.pdo.lock, &irql); + ext->u.pdo.removed = TRUE; + KeReleaseSpinLock(&ext->u.pdo.lock, irql); + + remove_pending_irps(ext); + + SetEvent(ext->u.pdo.halt_event); status = STATUS_SUCCESS; break;
On 7/1/21 5:16 PM, Zebediah Figura (she/her) wrote:
diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index 5f59257cdf7..5d6fb3d068c 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -359,11 +359,24 @@ static NTSTATUS fdo_pnp(DEVICE_OBJECT *device, IRP *irp) } } +static void remove_pending_irps(BASE_DEVICE_EXTENSION *ext) +{ + IRP *irp; + + while ((irp = pop_irp_from_queue(ext))) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + } +} + static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) { IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); BASE_DEVICE_EXTENSION *ext = device->DeviceExtension; NTSTATUS status = irp->IoStatus.Status; + IRP *queued_irp; + KIRQL irql; TRACE("irp %p, minor function %#x.\n", irp, irpsp->MinorFunction); @@ -453,12 +466,14 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp)
IoSetDeviceInterfaceState(&ext->u.pdo.mouse_link_name, TRUE); if (ext->u.pdo.is_keyboard)
IoSetDeviceInterfaceState(&ext->u.pdo.keyboard_link_name, TRUE); + + ext->u.pdo.removed = FALSE; status = STATUS_SUCCESS; break; case IRP_MN_REMOVE_DEVICE: { - IRP *queued_irp; + remove_pending_irps(ext);
In that case there's no reason to delete queued IRPs later in the handler.
Well, according to [1] it's what we are supposed to do:
4. Prevent any new I/O operations on the device.
[...]
5. Fail outstanding I/O requests on the device.
[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/handling-an... -- Rémi Bernon <rbernon(a)codeweavers.com>
On 7/1/21 11:29 AM, Rémi Bernon wrote:
On 7/1/21 5:16 PM, Zebediah Figura (she/her) wrote:
diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index 5f59257cdf7..5d6fb3d068c 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -359,11 +359,24 @@ static NTSTATUS fdo_pnp(DEVICE_OBJECT *device, IRP *irp) } } +static void remove_pending_irps(BASE_DEVICE_EXTENSION *ext) +{ + IRP *irp; + + while ((irp = pop_irp_from_queue(ext))) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + } +} + static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) { IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); BASE_DEVICE_EXTENSION *ext = device->DeviceExtension; NTSTATUS status = irp->IoStatus.Status; + IRP *queued_irp; + KIRQL irql; TRACE("irp %p, minor function %#x.\n", irp, irpsp->MinorFunction); @@ -453,12 +466,14 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp)
IoSetDeviceInterfaceState(&ext->u.pdo.mouse_link_name, TRUE); if (ext->u.pdo.is_keyboard)
IoSetDeviceInterfaceState(&ext->u.pdo.keyboard_link_name, TRUE); + + ext->u.pdo.removed = FALSE; status = STATUS_SUCCESS; break; case IRP_MN_REMOVE_DEVICE: { - IRP *queued_irp; + remove_pending_irps(ext);
In that case there's no reason to delete queued IRPs later in the handler.
Well, according to [1] it's what we are supposed to do:
4. Prevent any new I/O operations on the device.
[...]
5. Fail outstanding I/O requests on the device.
As I understand step 4 corresponds to setting the 'removed' flag to TRUE. Also, I'm talking about IRP_MN_REMOVE_DEVICE here, and not IRP_MN_SURPRISE_REMOVAL. Currently we're failing outstanding I/O twice.
[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/handling-an...
On 7/1/21 6:43 PM, Zebediah Figura (she/her) wrote:
On 7/1/21 11:29 AM, Rémi Bernon wrote:
On 7/1/21 5:16 PM, Zebediah Figura (she/her) wrote:
diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index 5f59257cdf7..5d6fb3d068c 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -359,11 +359,24 @@ static NTSTATUS fdo_pnp(DEVICE_OBJECT *device, IRP *irp) } } +static void remove_pending_irps(BASE_DEVICE_EXTENSION *ext) +{ + IRP *irp; + + while ((irp = pop_irp_from_queue(ext))) + { + irp->IoStatus.Status = STATUS_DELETE_PENDING; + IoCompleteRequest(irp, IO_NO_INCREMENT); + } +} + static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) { IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); BASE_DEVICE_EXTENSION *ext = device->DeviceExtension; NTSTATUS status = irp->IoStatus.Status; + IRP *queued_irp; + KIRQL irql; TRACE("irp %p, minor function %#x.\n", irp, irpsp->MinorFunction); @@ -453,12 +466,14 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp) IoSetDeviceInterfaceState(&ext->u.pdo.mouse_link_name, TRUE); if (ext->u.pdo.is_keyboard) IoSetDeviceInterfaceState(&ext->u.pdo.keyboard_link_name, TRUE); + + ext->u.pdo.removed = FALSE; status = STATUS_SUCCESS; break; case IRP_MN_REMOVE_DEVICE: { - IRP *queued_irp; + remove_pending_irps(ext);
In that case there's no reason to delete queued IRPs later in the handler.
Well, according to [1] it's what we are supposed to do:
4. Prevent any new I/O operations on the device.
[...]
5. Fail outstanding I/O requests on the device.
As I understand step 4 corresponds to setting the 'removed' flag to TRUE. Also, I'm talking about IRP_MN_REMOVE_DEVICE here, and not IRP_MN_SURPRISE_REMOVAL. Currently we're failing outstanding I/O twice.
Oh right, I thought this was referring the next hunk. -- Rémi Bernon <rbernon(a)codeweavers.com>
Instead of sending both IRP_MN_SURPRISE_REMOVAL and IRP_MN_REMOVE_DEVICE to all children first. They may have pending IRPs sent to their parent PDO driver, which need to be cancelled before IRP_MN_REMOVE_DEVICE can complete. This is the case for hidclass.sys and its thread calling the lower driver for instance. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/pnp.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index e425712d738..d8eb62bc17a 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -350,21 +350,20 @@ static void enumerate_new_device( DEVICE_OBJECT *device, HDEVINFO set ) start_device( device, set, &sp_device ); } -static void remove_device( DEVICE_OBJECT *device ) +static void send_remove_device_irp( DEVICE_OBJECT *device, UCHAR code ) { struct wine_device *wine_device = CONTAINING_RECORD(device, struct wine_device, device_obj); - TRACE("Removing device %p.\n", device); + TRACE( "Removing device %p, code %x.\n", device, code ); if (wine_device->children) { ULONG i; for (i = 0; i < wine_device->children->Count; ++i) - remove_device( wine_device->children->Objects[i] ); + send_remove_device_irp( wine_device->children->Objects[i], code ); } - send_pnp_irp( device, IRP_MN_SURPRISE_REMOVAL ); - send_pnp_irp( device, IRP_MN_REMOVE_DEVICE ); + send_pnp_irp( device, code ); } static BOOL device_in_list( const DEVICE_RELATIONS *list, const DEVICE_OBJECT *device ) @@ -441,7 +440,8 @@ static void handle_bus_relations( DEVICE_OBJECT *parent ) if (!device_in_list( relations, child )) { TRACE("Removing device %p.\n", child); - remove_device( child ); + send_remove_device_irp( child, IRP_MN_SURPRISE_REMOVAL ); + send_remove_device_irp( child, IRP_MN_REMOVE_DEVICE ); } ObDereferenceObject( child ); } @@ -1105,7 +1105,10 @@ void pnp_manager_stop_driver( struct wine_driver *driver ) struct root_pnp_device *device, *next; LIST_FOR_EACH_ENTRY_SAFE( device, next, &driver->root_pnp_devices, struct root_pnp_device, entry ) - remove_device( device->device ); + { + send_remove_device_irp( device->device, IRP_MN_SURPRISE_REMOVAL ); + send_remove_device_irp( device->device, IRP_MN_REMOVE_DEVICE ); + } } void pnp_manager_stop(void) @@ -1179,7 +1182,8 @@ void CDECL wine_enumerate_root_devices( const WCHAR *driver_name ) { TRACE("Removing device %s.\n", debugstr_w(pnp_device->id)); - remove_device( pnp_device->device ); + send_remove_device_irp( pnp_device->device, IRP_MN_SURPRISE_REMOVAL ); + send_remove_device_irp( pnp_device->device, IRP_MN_REMOVE_DEVICE ); } list_move_head( &driver->root_pnp_devices, &new_list ); -- 2.32.0
participants (2)
-
Rémi Bernon -
Zebediah Figura (she/her)