Re: [win-pv-devel] [PATCH] Clear CSQ before zeroing it during IRP_MJ_CLEANUP

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf Of owen.smith@xxxxxxxxxx
> Sent: 22 August 2017 12:37
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH] Clear CSQ before zeroing it during
> IRP_MJ_CLEANUP
> 
> From: Owen Smith <owen.smith@xxxxxxxxxx>
> 
> Without cancelling all IRPs in the cancel safe queue, xencons would
> bugcheck (0xCC) after completing IRP_MJ_CLEANUP.
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>  src/xencons/fdo.c    | 29 ++++++++++++++++++++++++++---
>  src/xencons/stream.c | 18 ++++++++++++++++--
>  2 files changed, 42 insertions(+), 5 deletions(-)
> 
> diff --git a/src/xencons/fdo.c b/src/xencons/fdo.c
> index 5992776..d114a60 100644
> --- a/src/xencons/fdo.c
> +++ b/src/xencons/fdo.c
> @@ -72,6 +72,7 @@ typedef struct _FDO_RESOURCE {
>  typedef struct _FDO_HANDLE {
>      LIST_ENTRY      ListEntry;
>      PFILE_OBJECT    FileObject;
> +    IO_REMOVE_LOCK  RemoveLock;
>      PXENCONS_STREAM Stream;
>  } FDO_HANDLE, *PFDO_HANDLE;
> 
> @@ -2114,6 +2115,10 @@ FdoCreateHandle(
>          goto fail2;
> 
>      Handle->FileObject = FileObject;
> +    IoInitializeRemoveLock(&Handle->RemoveLock,
> +                           FDO_POOL,
> +                           0,
> +                           0);
> 
>      KeAcquireSpinLock(&Fdo->HandleLock, &Irql);
>      InsertTailList(&Fdo->HandleList, &Handle->ListEntry);
> @@ -2178,10 +2183,12 @@ fail1:
>  static VOID
>  FdoDestroyHandle(
>      IN  PXENCONS_FDO    Fdo,
> -    IN  PFDO_HANDLE     Handle
> +    IN  PFDO_HANDLE     Handle,
> +    IN  PIRP            Irp
>      )
>  {
>      KIRQL               Irql;
> +    NTSTATUS            status;
> 
>      KeAcquireSpinLock(&Fdo->HandleLock, &Irql);
>      RemoveEntryList(&Handle->ListEntry);
> @@ -2191,6 +2198,12 @@ FdoDestroyHandle(
> 
>      Trace("%p\n", Handle->FileObject);
> 
> +    status = IoAcquireRemoveLock(&Handle->RemoveLock, Irp);
> +    ASSERT(NT_SUCCESS(status));
> +    IoReleaseRemoveLockAndWait(&Handle->RemoveLock, Irp);
> +
> +    RtlZeroMemory(&Handle->RemoveLock, sizeof(IO_REMOVE_LOCK));
> +
>      StreamDestroy(Handle->Stream);
>      Handle->Stream = NULL;
> 
> @@ -2237,7 +2250,7 @@ FdoDispatchCleanup(
>      if (Handle == NULL)
>          goto fail1;
> 
> -    FdoDestroyHandle(Fdo, Handle);
> +    FdoDestroyHandle(Fdo, Handle, Irp);
>      status = STATUS_SUCCESS;
> 
>      Irp->IoStatus.Status = status;
> @@ -2292,12 +2305,22 @@ FdoDispatchReadWrite(
> 
>      IoMarkIrpPending(Irp);
> 
> -    status = StreamPutQueue(Handle->Stream, Irp);
> +    status = IoAcquireRemoveLock(&Handle->RemoveLock, Irp);
>      if (!NT_SUCCESS(status))
>          goto fail2;
> 
> +    status = StreamPutQueue(Handle->Stream, Irp);
> +    if (!NT_SUCCESS(status))
> +        goto fail3;
> +
> +    IoReleaseRemoveLock(&Handle->RemoveLock, Irp);
>      return STATUS_PENDING;
> 
> +fail3:
> +    Error("fail3\n");
> +
> +    IoReleaseRemoveLock(&Handle->RemoveLock, Irp);
> +
>  fail2:
>      Error("fail2\n");
> 
> diff --git a/src/xencons/stream.c b/src/xencons/stream.c
> index 0f4c129..e447a8e 100644
> --- a/src/xencons/stream.c
> +++ b/src/xencons/stream.c
> @@ -46,7 +46,7 @@ struct _XENCONS_STREAM {
>      PXENCONS_THREAD          Thread;
>      IO_CSQ                   Csq;
>      LIST_ENTRY               List;
> -    KSPIN_LOCK               Lock;
> +    KSPIN_LOCK                   Lock;

Spurious whitespace change.

>      XENBUS_CONSOLE_INTERFACE         ConsoleInterface;
>  };
> 
> @@ -188,7 +188,7 @@ StreamCsqCompleteCanceledIrp(
>      Irp->IoStatus.Information = 0;
>      Irp->IoStatus.Status = STATUS_CANCELLED;
> 
> -    Trace("COMPLETE (%02x:%s)\n",
> +    Trace("CANCELLED (%02x:%s)\n",
>            MajorFunction,
>            MajorFunctionName(MajorFunction));
> 
> @@ -418,6 +418,20 @@ StreamDestroy(
>      ThreadJoin(Stream->Thread);
>      Stream->Thread = NULL;
> 
> +    // empty the queue before zeroing the CSQ and List
> +    for (;;) {
> +        PIRP            Irp;
> +
> +        Irp = IoCsqRemoveNextIrp(&Stream->Csq, NULL);
> +        if (Irp == NULL)
> +            break;
> +
> +        StreamCsqCompleteCanceledIrp(&Stream->Csq,
> +                                     Irp);
> +    }
> +
> +    ASSERT(IsListEmpty(&Stream->List));
> +
>      RtlZeroMemory(&Stream->Csq, sizeof (IO_CSQ));
> 
>      RtlZeroMemory(&Stream->List, sizeof (LIST_ENTRY));
> --
> 2.8.3
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel