Re: [PATCH 1/3] hidclass.sys: Don't leak report queues on device removal.

[Aric Stewart]

Signed-off-by: Aric Stewart

On 9/24/21 3:07 AM, Rémi Bernon wrote:
> Signed-off-by: Rémi Bernon
>
> dlls/hidclass.sys/device.c | 27 ++++++++++++++++++++++++++-
>   dlls/hidclass.sys/hid.h    |  2 +-
>   dlls/hidclass.sys/pnp.c    |  6 ++++++
>   3 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/hidclass.sys/device.c b/dlls/hidclass.sys/device.c
> index 848f8da906a..21941a1802e 100644
> --- a/dlls/hidclass.sys/device.c
> +++ b/dlls/hidclass.sys/device.c
> @@ -116,9 +116,10 @@ static struct hid_report_queue *hid_report_queue_create( void )
>       return queue;
>   }
>
> -static void hid_report_queue_destroy( struct hid_report_queue *queue )
> +void hid_report_queue_destroy( struct hid_report_queue *queue )
>   {
>       while (queue->length--) hid_report_decref( queue->reports[queue->length] );
> - list_remove( &queue->entry );
>  free( queue );
> }
>
> @@ -654,10 +655,22 @@ NTSTATUS WINAPI pdo_create(DEVICE_OBJECT *device, IRP *irp)
>   {
>       BASE_DEVICE_EXTENSION *ext = device->DeviceExtension;
>       struct hid_report_queue *queue;
> -
>
> BOOL removed;
>  KIRQL irql;
>
> TRACE("Open handle on device %p\n", device);
> -
>
> 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;
> -
>
> }
> -
> if (!(queue = hid_report_queue_create())) irp->IoStatus.Status = STATUS_NO_MEMORY;
> else
> {
>
> @@ -677,10 +690,22 @@ NTSTATUS WINAPI pdo_close(DEVICE_OBJECT *device, IRP *irp)
>   {
>       struct hid_report_queue *queue = irp->Tail.Overlay.OriginalFileObject->FsContext;
>       BASE_DEVICE_EXTENSION *ext = device->DeviceExtension;
> -
>
> BOOL removed;
>  KIRQL irql;
>
> TRACE("Close handle on device %p\n", device);
> -
>
> 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;
> -
>
> }
> -
> if (queue)
> {
>     KeAcquireSpinLock( &ext->u.pdo.report_queues_lock, &irql );
>
> diff --git a/dlls/hidclass.sys/hid.h b/dlls/hidclass.sys/hid.h
> index e65fabb2aea..60f3d0fb57e 100644
> --- a/dlls/hidclass.sys/hid.h
> +++ b/dlls/hidclass.sys/hid.h
> @@ -123,7 +123,7 @@ void call_minidriver( ULONG code, DEVICE_OBJECT *device, void *in_buff, ULONG in
>
> /* Internal device functions */
>   void HID_StartDeviceThread(DEVICE_OBJECT *device) DECLSPEC_HIDDEN;
> -
>
> +void hid_report_queue_destroy( struct hid_report_queue *queue );
>   IRP *pop_irp_from_queue(BASE_DEVICE_EXTENSION *ext) DECLSPEC_HIDDEN;
>
> NTSTATUS WINAPI pdo_ioctl(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN;
> diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c
> index 8755afbce6c..8c0c8adafe6 100644
> --- a/dlls/hidclass.sys/pnp.c
> +++ b/dlls/hidclass.sys/pnp.c
> @@ -387,6 +387,7 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp)
>       BASE_DEVICE_EXTENSION *ext = device->DeviceExtension;
>       HIDP_COLLECTION_DESC *desc = ext->u.pdo.device_desc.CollectionDesc;
>       NTSTATUS status = irp->IoStatus.Status;
> -
>
> struct hid_report_queue *queue, *next;
>  KIRQL irql;
>
> TRACE("irp %p, minor function %#x.\n", irp, irpsp->MinorFunction);
>
> @@ -498,6 +499,11 @@ static NTSTATUS pdo_pnp(DEVICE_OBJECT *device, IRP *irp)
>               }
>               CloseHandle(ext->u.pdo.halt_event);
> -
>        KeAcquireSpinLock( &ext->u.pdo.report_queues_lock, &irql );
> -
>        LIST_FOR_EACH_ENTRY_SAFE( queue, next, &ext->u.pdo.report_queues, struct hid_report_queue, entry )
> -
>            hid_report_queue_destroy( queue );
> -
>        KeReleaseSpinLock( &ext->u.pdo.report_queues_lock, irql );
> -
>         HidP_FreeCollectionDescription(&ext->u.pdo.device_desc);
>
>         RtlFreeUnicodeString(&ext->u.pdo.link_name);