Re: [PATCH v2 2/5] ntoskrnl.exe: Implement Ke(Initialize|Insert|Remove)DeviceQueue.

[Zebediah Figura (she/her)]

Sorry I didn't notice this before, but...

On 6/29/21 2:21 AM, Rémi Bernon wrote:
> +BOOLEAN WINAPI KeInsertDeviceQueue( KDEVICE_QUEUE *queue, KDEVICE_QUEUE_ENTRY *entry )
> +{
> - KIRQL irql;
> -
> - TRACE( "queue %p, entry %p.\n", queue, entry );
> -
> - KeAcquireSpinLock( &queue->Lock, &irql );
> - if ((entry->Inserted = queue->Busy))
> -
>    InsertTailList( &queue->DeviceListHead, &entry->DeviceListEntry );
> - queue->Busy = TRUE;
> - KeReleaseSpinLock( &queue->Lock, irql );
> -
> - return entry->Inserted;
>
> +}

I don't think it's thread-safe to access entry->Inserted outside of the 
lock.
> -
>
> +KDEVICE_QUEUE_ENTRY *WINAPI KeRemoveDeviceQueue( KDEVICE_QUEUE *queue )
> +{
> - LIST_ENTRY *entry = NULL;
> - KIRQL irql;
> -
> - TRACE( "queue %p.\n", queue );
> -
> - KeAcquireSpinLock( &queue->Lock, &irql );
> - if (IsListEmpty( &queue->DeviceListHead )) queue->Busy = FALSE;
> - else entry = RemoveHeadList( &queue->DeviceListHead );
> - KeReleaseSpinLock( &queue->Lock, irql );
> -
> - if (!entry) return NULL;
> - return CONTAINING_RECORD( entry, KDEVICE_QUEUE_ENTRY, DeviceListEntry );
>
> +}

And while we're at it, should this unset entry->Inserted?