From 43273addf5aa21354bae6685490f105584b03440 Mon Sep 17 00:00:00 2001 From: Thomas Faber Date: Mon, 23 Jul 2012 13:24:48 +0000 Subject: [PATCH] [NPFS] - Keep a reference count for FCBs to prevent race conditions when closing multiple handles to the same pipe concurrently - Acquire the pipe list lock in NpfsQueryDirectory to guard against concurrent deletion of pipes See issue #7205 for more details. svn path=/trunk/; revision=56950 --- reactos/drivers/filesystems/npfs/create.c | 44 +++++++++++------------ reactos/drivers/filesystems/npfs/dirctl.c | 8 +++++ reactos/drivers/filesystems/npfs/fsctrl.c | 10 ++++-- reactos/drivers/filesystems/npfs/npfs.c | 2 ++ reactos/drivers/filesystems/npfs/npfs.h | 7 ++-- 5 files changed, 43 insertions(+), 28 deletions(-) diff --git a/reactos/drivers/filesystems/npfs/create.c b/reactos/drivers/filesystems/npfs/create.c index 3be21ad2879..8d7a342fcc1 100644 --- a/reactos/drivers/filesystems/npfs/create.c +++ b/reactos/drivers/filesystems/npfs/create.c @@ -17,17 +17,20 @@ /* FUNCTIONS *****************************************************************/ -static VOID -NpfsDeleteFcb(PNPFS_FCB Fcb) +NpfsDereferenceFcb(PNPFS_FCB Fcb) { PNPFS_VCB Vcb = Fcb->Vcb; KeLockMutex(&Vcb->PipeListLock); - RemoveEntryList(&Fcb->PipeListEntry); + if (InterlockedDecrement(&Fcb->RefCount) == 0) + { + DPRINT("NpfsDereferenceFcb. Deleting %p\n", Fcb); + RemoveEntryList(&Fcb->PipeListEntry); + RtlFreeUnicodeString(&Fcb->PipeName); + ExFreePoolWithTag(Fcb, TAG_NPFS_FCB); + } KeUnlockMutex(&Vcb->PipeListLock); - RtlFreeUnicodeString(&Fcb->PipeName); - ExFreePoolWithTag(Fcb, TAG_NPFS_FCB); } static @@ -103,6 +106,7 @@ NpfsFindPipe(PNPFS_VCB Vcb, TRUE) == 0) { DPRINT("<%wZ> = <%wZ>\n", PipeName, &Fcb->PipeName); + (VOID)InterlockedIncrement(&Fcb->RefCount); return Fcb; } @@ -337,6 +341,7 @@ NpfsCreate(PDEVICE_OBJECT DeviceObject, { DPRINT("No memory!\n"); KeUnlockMutex(&Fcb->CcbListLock); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_NO_MEMORY; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_NO_MEMORY; @@ -365,6 +370,7 @@ NpfsCreate(PDEVICE_OBJECT DeviceObject, DPRINT("No memory!\n"); NpfsDereferenceCcb(ClientCcb); KeUnlockMutex(&Fcb->CcbListLock); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_NO_MEMORY; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_NO_MEMORY; @@ -437,6 +443,7 @@ NpfsCreate(PDEVICE_OBJECT DeviceObject, NpfsDereferenceCcb(ClientCcb); KeUnlockMutex(&Fcb->CcbListLock); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_OBJECT_PATH_NOT_FOUND; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_OBJECT_PATH_NOT_FOUND; @@ -460,8 +467,8 @@ NpfsCreate(PDEVICE_OBJECT DeviceObject, } NpfsDereferenceCcb(ClientCcb); - KeUnlockMutex(&Fcb->CcbListLock); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_UNSUCCESSFUL; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_UNSUCCESSFUL; @@ -510,7 +517,6 @@ NpfsCreateNamedPipe(PDEVICE_OBJECT DeviceObject, PNPFS_FCB Fcb; PNPFS_CCB Ccb; PNAMED_PIPE_CREATE_PARAMETERS Buffer; - BOOLEAN NewPipe = FALSE; DPRINT("NpfsCreateNamedPipe(DeviceObject %p Irp %p)\n", DeviceObject, Irp); @@ -549,6 +555,7 @@ NpfsCreateNamedPipe(PDEVICE_OBJECT DeviceObject, if (Fcb->CurrentInstances >= Fcb->MaximumInstances) { DPRINT("Out of instances.\n"); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_INSTANCE_NOT_AVAILABLE; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_INSTANCE_NOT_AVAILABLE; @@ -559,6 +566,7 @@ NpfsCreateNamedPipe(PDEVICE_OBJECT DeviceObject, Fcb->PipeType != Buffer->NamedPipeType) { DPRINT("Asked for invalid pipe mode.\n"); + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_ACCESS_DENIED; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_ACCESS_DENIED; @@ -566,7 +574,6 @@ NpfsCreateNamedPipe(PDEVICE_OBJECT DeviceObject, } else { - NewPipe = TRUE; Fcb = ExAllocatePoolWithTag(NonPagedPool, sizeof(NPFS_FCB), TAG_NPFS_FCB); if (Fcb == NULL) { @@ -579,6 +586,7 @@ NpfsCreateNamedPipe(PDEVICE_OBJECT DeviceObject, Fcb->Type = FCB_PIPE; Fcb->Vcb = Vcb; + Fcb->RefCount = 1; Fcb->PipeName.Length = FileObject->FileName.Length; Fcb->PipeName.MaximumLength = Fcb->PipeName.Length + sizeof(UNICODE_NULL); Fcb->PipeName.Buffer = ExAllocatePoolWithTag(NonPagedPool, @@ -678,11 +686,7 @@ NpfsCreateNamedPipe(PDEVICE_OBJECT DeviceObject, Ccb = NpfsAllocateCcb(CCB_PIPE, Fcb); if (Ccb == NULL) { - if (NewPipe) - { - NpfsDeleteFcb(Fcb); - } - + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_NO_MEMORY; IoCompleteRequest(Irp, IO_NO_INCREMENT); return STATUS_NO_MEMORY; @@ -698,11 +702,7 @@ NpfsCreateNamedPipe(PDEVICE_OBJECT DeviceObject, if (Ccb->Data == NULL) { NpfsDereferenceCcb(Ccb); - - if (NewPipe) - { - NpfsDeleteFcb(Fcb); - } + NpfsDereferenceFcb(Fcb); Irp->IoStatus.Status = STATUS_NO_MEMORY; IoCompleteRequest(Irp, IO_NO_INCREMENT); @@ -1004,12 +1004,8 @@ NpfsClose(PDEVICE_OBJECT DeviceObject, KeUnlockMutex(&Fcb->CcbListLock); - if (IsListEmpty(&Fcb->ServerCcbListHead) && - IsListEmpty(&Fcb->ClientCcbListHead)) - { - NpfsDeleteFcb(Fcb); - FileObject->FsContext = NULL; - } + NpfsDereferenceFcb(Fcb); + FileObject->FsContext = NULL; Irp->IoStatus.Status = STATUS_SUCCESS; Irp->IoStatus.Information = 0; diff --git a/reactos/drivers/filesystems/npfs/dirctl.c b/reactos/drivers/filesystems/npfs/dirctl.c index e73b0424ded..17039b20faa 100644 --- a/reactos/drivers/filesystems/npfs/dirctl.c +++ b/reactos/drivers/filesystems/npfs/dirctl.c @@ -144,6 +144,7 @@ NpfsQueryDirectory(PNPFS_CCB Ccb, PipeIndex = 0; Vcb = Ccb->Fcb->Vcb; + KeLockMutex(&Vcb->PipeListLock); CurrentEntry = Vcb->PipeListHead.Flink; while (CurrentEntry != &Vcb->PipeListHead && Status == STATUS_SUCCESS) @@ -252,11 +253,17 @@ NpfsQueryDirectory(PNPFS_CCB Ccb, /* Leave, if there is no space left in the buffer */ if (Status == STATUS_BUFFER_OVERFLOW) + { + KeUnlockMutex(&Vcb->PipeListLock); return Status; + } /* Leave, if we should return only one entry */ if (Stack->Flags & SL_RETURN_SINGLE_ENTRY) + { + KeUnlockMutex(&Vcb->PipeListLock); return STATUS_SUCCESS; + } /* Store the current offset for the next round */ LastOffset = CurrentOffset; @@ -270,6 +277,7 @@ NpfsQueryDirectory(PNPFS_CCB Ccb, CurrentEntry = CurrentEntry->Flink; } + KeUnlockMutex(&Vcb->PipeListLock); /* Return STATUS_NO_MORE_FILES if no matching pipe name was found */ if (CurrentOffset == 0) diff --git a/reactos/drivers/filesystems/npfs/fsctrl.c b/reactos/drivers/filesystems/npfs/fsctrl.c index 394f5b2987a..9a2bb6ad426 100644 --- a/reactos/drivers/filesystems/npfs/fsctrl.c +++ b/reactos/drivers/filesystems/npfs/fsctrl.c @@ -375,7 +375,7 @@ NpfsWaitPipe(PIRP Irp, { /* found a listening server CCB */ DPRINT("Listening server CCB found -- connecting\n"); - + NpfsDereferenceFcb(Fcb); return STATUS_SUCCESS; } @@ -402,6 +402,7 @@ NpfsWaitPipe(PIRP Irp, /* Wait forever */ TimeOut = NULL; } + NpfsDereferenceFcb(Fcb); Status = KeWaitForSingleObject(&Ccb->ConnectEvent, UserRequest, @@ -507,7 +508,9 @@ NpfsWaitPipe2(PIRP Irp, { /* found a listening server CCB */ DPRINT("Listening server CCB found -- connecting\n"); - +#ifdef USING_PROPER_NPFS_WAIT_SEMANTICS + NpfsDereferenceFcb(Fcb); +#endif return STATUS_SUCCESS; } @@ -521,6 +524,9 @@ NpfsWaitPipe2(PIRP Irp, TimeOut = WaitPipe->Timeout; else TimeOut = Fcb->TimeOut; +#ifdef USING_PROPER_NPFS_WAIT_SEMANTICS + NpfsDereferenceFcb(Fcb); +#endif /* Wait for one */ Status = KeWaitForSingleObject(&Ccb->ConnectEvent, diff --git a/reactos/drivers/filesystems/npfs/npfs.c b/reactos/drivers/filesystems/npfs/npfs.c index 58b0ffcee3b..13f92ab7554 100644 --- a/reactos/drivers/filesystems/npfs/npfs.c +++ b/reactos/drivers/filesystems/npfs/npfs.c @@ -89,12 +89,14 @@ DriverEntry(PDRIVER_OBJECT DriverObject, Fcb = ExAllocatePoolWithTag(NonPagedPool, sizeof(NPFS_FCB), TAG_NPFS_FCB); Fcb->Type = FCB_DEVICE; Fcb->Vcb = Vcb; + Fcb->RefCount = 1; Vcb->DeviceFcb = Fcb; /* Create the root directory FCB */ Fcb = ExAllocatePoolWithTag(NonPagedPool, sizeof(NPFS_FCB), TAG_NPFS_FCB); Fcb->Type = FCB_DIRECTORY; Fcb->Vcb = Vcb; + Fcb->RefCount = 1; Vcb->RootFcb = Fcb; return STATUS_SUCCESS; diff --git a/reactos/drivers/filesystems/npfs/npfs.h b/reactos/drivers/filesystems/npfs/npfs.h index dd984851ac7..31589e2bf39 100644 --- a/reactos/drivers/filesystems/npfs/npfs.h +++ b/reactos/drivers/filesystems/npfs/npfs.h @@ -51,6 +51,7 @@ typedef struct _NPFS_FCB { FCB_TYPE Type; PNPFS_VCB Vcb; + volatile LONG RefCount; UNICODE_STRING PipeName; LIST_ENTRY PipeListEntry; KMUTEX CcbListLock; @@ -83,8 +84,7 @@ typedef struct _NPFS_CCB LIST_ENTRY CcbListEntry; CCB_TYPE Type; PNPFS_FCB Fcb; - - PFILE_OBJECT FileObject; + PFILE_OBJECT FileObject; struct _NPFS_CCB* OtherSide; struct ETHREAD *Thread; @@ -190,6 +190,9 @@ NTSTATUS NTAPI DriverEntry(PDRIVER_OBJECT DriverObject, PUNICODE_STRING RegistryPath); +VOID +NpfsDereferenceFcb(PNPFS_FCB Fcb); + PNPFS_FCB NpfsFindPipe(PNPFS_VCB Vcb, PUNICODE_STRING PipeName);