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?